6:23 a.m.
l_dbus_remove_watch (_dbus_name_cache_remove_watch) marks name watches
as removed and schedules a service_watch_remove_all call. Do the
watch's destroy callback when marking the watch as removed instead of
waiting until the service_watch_remove_all idle callback to do this.
Semantically it may be ok to do the destroy callback later but I think
it's easier on the user if they can be sure they'll get no more
callbacks after using l_dbus_remove_watch.
---
ell/dbus-name-cache.c | 6 ++++++
1 file changed, 6 insertions(+)
diff --git a/ell/dbus-name-cache.c b/ell/dbus-name-cache.c
index cb6a516..b09cc97 100644
--- a/ell/dbus-name-cache.c
+++ b/ell/dbus-name-cache.c
@@ -283,6 +283,12 @@ static void service_watch_mark(const void *key, void *value, void
*user_data)
watch->removed = true;
watch->connect_func = NULL;
watch->disconnect_func = NULL;
+
+ if (watch->destroy) {
+ watch->destroy(watch->user_data);
+ watch->destroy = NULL;
+ }
+
*id = 0;
break;
}
--
2.25.1
6:23 a.m.
New subject: [PATCH 2/2] dbus: Replace service_watch.removed by zeroing .id
l_dbus_remove_watch would set service_watch::removed true and return
success, but another call to l_dbus_remove_watch would still succeed
because we weren't checking if removed was already true. We could add
that check but we can also zero the id instead and simplify the struct
slightly.
---
This is almost a cosmetic change but I think it may be in the right
direction, feel free to ignore it if you think otherwise.
---
ell/dbus-name-cache.c | 8 +++++---
1 file changed, 5 insertions(+), 3 deletions(-)
diff --git a/ell/dbus-name-cache.c b/ell/dbus-name-cache.c
index b09cc97..000cdb5 100644
--- a/ell/dbus-name-cache.c
+++ b/ell/dbus-name-cache.c
@@ -48,7 +48,6 @@ struct service_watch {
l_dbus_destroy_func_t destroy;
void *user_data;
unsigned int id;
- bool removed;
struct service_watch *next;
};
@@ -241,7 +240,7 @@ static bool service_watch_remove(const void *key, void *value, void
*user_data)
struct service_watch **watch, *tmp;
for (watch = &entry->watches; *watch;) {
- if (!(*watch)->removed) {
+ if ((*watch)->id) {
watch = &(*watch)->next;
continue;
}
@@ -278,9 +277,12 @@ static void service_watch_mark(const void *key, void *value, void
*user_data)
struct service_watch *watch;
unsigned int *id = user_data;
+ if (!*id)
+ return;
+
for (watch = entry->watches; watch; watch = watch->next)
if (watch->id == *id) {
- watch->removed = true;
+ watch->id = 0;
watch->connect_func = NULL;
watch->disconnect_func = NULL;
--
2.25.1
9:26 a.m.
Hi Andrew,
On 4/30/20 8:23 AM, Andrew Zaborowski wrote:
l_dbus_remove_watch (_dbus_name_cache_remove_watch) marks name
watches
as removed and schedules a service_watch_remove_all call. Do the
watch's destroy callback when marking the watch as removed instead of
waiting until the service_watch_remove_all idle callback to do this.
Semantically it may be ok to do the destroy callback later but I think
it's easier on the user if they can be sure they'll get no more
callbacks after using l_dbus_remove_watch.
---
ell/dbus-name-cache.c | 6 ++++++
1 file changed, 6 insertions(+)
Both applied, thanks.
Regards,
-Denis
181
days inactive
182
days old
2 comments
2 participants
participants (2)
-
Andrew Zaborowski -
Denis Kenzior