At the moment if you have multiple notifiers it becomes impossible to figure out when it is safe to free the values.
This new callback can only be added once and is called once, making it better suited to freeing memory. It is also called after the deleted and replaced notifiers.
Signed-off-by: Angus Salkeld asalkeld@redhat.com --- include/qb/qbmap.h | 1 + lib/hashtable.c | 23 ++++++++++++++++++- lib/map.c | 3 ++ lib/skiplist.c | 21 ++++++++++++++++- lib/trie.c | 31 ++++++++++++++++++++++++- tests/check_map.c | 62 ++++++++++++++++++++++++++++++++++++++++++++++++++++ 6 files changed, 137 insertions(+), 4 deletions(-)
diff --git a/include/qb/qbmap.h b/include/qb/qbmap.h index dc32a56..18a08a1 100644 --- a/include/qb/qbmap.h +++ b/include/qb/qbmap.h @@ -48,6 +48,7 @@ typedef struct qb_map_iter qb_map_iter_t; #define QB_MAP_NOTIFY_REPLACED 2 #define QB_MAP_NOTIFY_INSERTED 4 #define QB_MAP_NOTIFY_RECURSIVE 8 +#define QB_MAP_NOTIFY_FREE 16
typedef void (*qb_map_notify_fn)(uint32_t event, char* key, diff --git a/lib/hashtable.c b/lib/hashtable.c index f6f2edf..492cc88 100644 --- a/lib/hashtable.c +++ b/lib/hashtable.c @@ -242,6 +242,12 @@ hashtable_notify(struct hash_table *t, struct hash_node *n, tn->callback(event, (char *)key, old_value, value, tn->user_data); } + if (((event & QB_MAP_NOTIFY_DELETED) || + (event & QB_MAP_NOTIFY_REPLACED)) && + (tn->events & QB_MAP_NOTIFY_FREE)) { + tn->callback(QB_MAP_NOTIFY_FREE, (char *)key, + old_value, value, tn->user_data); + } } }
@@ -254,6 +260,7 @@ hashtable_notify_add(qb_map_t * m, const char *key, struct hash_node *n; struct qb_list_head *head = NULL; struct qb_list_head *list; + int add_to_tail = QB_FALSE;
if (key) { n = hashtable_lookup(t, key); @@ -266,9 +273,18 @@ hashtable_notify_add(qb_map_t * m, const char *key, if (head == NULL) { return -ENOENT; } + if (events & QB_MAP_NOTIFY_FREE) { + add_to_tail = QB_TRUE; + } + for (list = head->next; list != head; list = list->next) { f = qb_list_entry(list, struct qb_map_notifier, list);
+ if (events & QB_MAP_NOTIFY_FREE && + f->events == events) { + /* only one free notifier */ + return -EEXIST; + } if (f->events == events && f->user_data == user_data && f->callback == fn) { @@ -284,7 +300,12 @@ hashtable_notify_add(qb_map_t * m, const char *key, f->user_data = user_data; f->callback = fn; qb_list_init(&f->list); - qb_list_add(&f->list, head); + + if (add_to_tail) { + qb_list_add_tail(&f->list, head); + } else { + qb_list_add(&f->list, head); + } return 0; }
diff --git a/lib/map.c b/lib/map.c index 2e28451..d3aa8f5 100644 --- a/lib/map.c +++ b/lib/map.c @@ -92,6 +92,9 @@ int32_t qb_map_notify_add(qb_map_t * m, const char *key, qb_map_notify_fn fn, int32_t events, void *user_data) { + if (key != NULL && events & QB_MAP_NOTIFY_FREE) { + return -EINVAL; + } if (m->notify_add) { return m->notify_add(m, key, fn, events, user_data); } else { diff --git a/lib/skiplist.c b/lib/skiplist.c index 045fa11..ff12c02 100644 --- a/lib/skiplist.c +++ b/lib/skiplist.c @@ -210,6 +210,12 @@ skiplist_notify(struct skiplist *l, struct skiplist_node *n, tn->callback(event, key, old_value, value, tn->user_data); } + if (((event & QB_MAP_NOTIFY_DELETED) || + (event & QB_MAP_NOTIFY_REPLACED)) && + (tn->events & QB_MAP_NOTIFY_FREE)) { + tn->callback(QB_MAP_NOTIFY_FREE, (char *)key, + old_value, value, tn->user_data); + } }
} @@ -242,17 +248,26 @@ skiplist_notify_add(qb_map_t * m, const char *key, struct qb_map_notifier *f; struct skiplist_node *n; struct qb_list_head *list; + int add_to_tail = QB_FALSE;
if (key) { n = skiplist_lookup(t, key); } else { n = t->header; } + if (events & QB_MAP_NOTIFY_FREE) { + add_to_tail = QB_TRUE; + } if (n) { for (list = n->notifier_head.next; list != &n->notifier_head; list = list->next) { f = qb_list_entry(list, struct qb_map_notifier, list);
+ if (events & QB_MAP_NOTIFY_FREE && + f->events == events) { + /* only one free notifier */ + return -EEXIST; + } if (f->events == events && f->callback == fn && f->user_data == user_data) { @@ -268,7 +283,11 @@ skiplist_notify_add(qb_map_t * m, const char *key, f->user_data = user_data; f->callback = fn; qb_list_init(&f->list); - qb_list_add(&f->list, &n->notifier_head); + if (add_to_tail) { + qb_list_add_tail(&f->list, &n->notifier_head); + } else { + qb_list_add(&f->list, &n->notifier_head); + } return 0; } return -EINVAL; diff --git a/lib/trie.c b/lib/trie.c index 3283d12..38dc2d1 100644 --- a/lib/trie.c +++ b/lib/trie.c @@ -122,6 +122,9 @@ keep_going: static void trie_node_destroy(struct trie *t, struct trie_node *n) { + if (t->header == n) { + return; + } trie_notify(n, QB_MAP_NOTIFY_DELETED, n->key, n->value, NULL);
n->key = NULL; @@ -131,6 +134,9 @@ trie_node_destroy(struct trie *t, struct trie_node *n) static void trie_node_deref(struct trie *t, struct trie_node *node) { + if (t->header == node) { + return; + } node->refcount--; if (node->refcount > 0) { return; @@ -297,6 +303,12 @@ trie_notify(struct trie_node *n, tn->callback(event, (char *)key, old_value, value, tn->user_data); } + if (((event & QB_MAP_NOTIFY_DELETED) || + (event & QB_MAP_NOTIFY_REPLACED)) && + (tn->events & QB_MAP_NOTIFY_FREE)) { + tn->callback(QB_MAP_NOTIFY_FREE, (char *)key, + old_value, value, tn->user_data); + } } c = c->parent; } while (c); @@ -310,6 +322,7 @@ trie_notify_add(qb_map_t * m, const char *key, struct qb_map_notifier *f; struct trie_node *n; struct qb_list_head *list; + int add_to_tail = QB_FALSE;
if (key) { n = trie_lookup(t, key, QB_TRUE); @@ -321,6 +334,11 @@ trie_notify_add(qb_map_t * m, const char *key, list != &n->notifier_head; list = list->next) { f = qb_list_entry(list, struct qb_map_notifier, list);
+ if (events & QB_MAP_NOTIFY_FREE && + f->events == events) { + /* only one free notifier */ + return -EEXIST; + } if (f->events == events && f->callback == fn && f->user_data == user_data) { @@ -336,10 +354,19 @@ trie_notify_add(qb_map_t * m, const char *key, f->user_data = user_data; f->callback = fn; qb_list_init(&f->list); - if (events & QB_MAP_NOTIFY_RECURSIVE) { - qb_list_add(&f->list, &n->notifier_head); + if (key) { + if (events & QB_MAP_NOTIFY_RECURSIVE) { + add_to_tail = QB_TRUE; + } } else { + if (events & QB_MAP_NOTIFY_FREE) { + add_to_tail = QB_TRUE; + } + } + if (add_to_tail) { qb_list_add_tail(&f->list, &n->notifier_head); + } else { + qb_list_add(&f->list, &n->notifier_head); } return 0; } diff --git a/tests/check_map.c b/tests/check_map.c index 815b321..011ad01 100644 --- a/tests/check_map.c +++ b/tests/check_map.c @@ -43,6 +43,8 @@ static char *notified_key = NULL; static void *notified_value = NULL; static void *notified_new_value = NULL; static void *notified_user_data = NULL; +static int32_t notified_event = 0; +static int32_t notified_event_prev = 0;
static void test_map_simple(qb_map_t *m, const char *name) @@ -190,6 +192,15 @@ my_map_notification(uint32_t event, notified_value = old_value; notified_new_value = value; notified_user_data = user_data; + notified_event_prev = notified_event; + notified_event = event; +} + +static void +my_map_notification_2(uint32_t event, + char* key, void* old_value, + void* value, void* user_data) +{ }
static void @@ -324,6 +335,51 @@ test_map_notifications_basic(qb_map_t *m) ck_assert_int_eq(i, -EEXIST); }
+/* test free'ing notifier + * + * input: + * only one can be added + * can only be added with NULL key (global) + * output: + * is the last notifier called (after deleted or replaced) + * recursive is implicit + */ +static void +test_map_notifications_free(qb_map_t *m) +{ + int32_t i; + i = qb_map_notify_add(m, "not global", my_map_notification, + QB_MAP_NOTIFY_FREE, m); + ck_assert_int_eq(i, -EINVAL); + i = qb_map_notify_add(m, NULL, my_map_notification, + QB_MAP_NOTIFY_FREE, m); + ck_assert_int_eq(i, 0); + i = qb_map_notify_add(m, NULL, my_map_notification_2, + QB_MAP_NOTIFY_FREE, m); + ck_assert_int_eq(i, -EEXIST); + i = qb_map_notify_del_2(m, NULL, my_map_notification, + QB_MAP_NOTIFY_FREE, m); + ck_assert_int_eq(i, 0); + i = qb_map_notify_add(m, NULL, my_map_notification, + (QB_MAP_NOTIFY_FREE | + QB_MAP_NOTIFY_REPLACED | + QB_MAP_NOTIFY_DELETED | + QB_MAP_NOTIFY_RECURSIVE), m); + ck_assert_int_eq(i, 0); + + qb_map_put(m, "garden", "grow"); + +/* update */ + qb_map_put(m, "garden", "green"); + ck_assert_int_eq(notified_event_prev, QB_MAP_NOTIFY_REPLACED); + ck_assert_int_eq(notified_event, QB_MAP_NOTIFY_FREE); + +/* delete */ + qb_map_rm(m, "garden"); + ck_assert_int_eq(notified_event_prev, QB_MAP_NOTIFY_DELETED); + ck_assert_int_eq(notified_event, QB_MAP_NOTIFY_FREE); +} + static void test_map_notifications_prefix(qb_map_t *m) { @@ -668,6 +724,8 @@ START_TEST(test_trie_notifications) test_map_notifications_basic(m); m = qb_trie_create(); test_map_notifications_prefix(m); + m = qb_trie_create(); + test_map_notifications_free(m); } END_TEST
@@ -676,6 +734,8 @@ START_TEST(test_hash_notifications) qb_map_t *m; m = qb_hashtable_create(256); test_map_notifications_basic(m); + m = qb_hashtable_create(256); + test_map_notifications_free(m); } END_TEST
@@ -684,6 +744,8 @@ START_TEST(test_skiplist_notifications) qb_map_t *m; m = qb_skiplist_create(); test_map_notifications_basic(m); + m = qb_skiplist_create(); + test_map_notifications_free(m); } END_TEST