From eaef130d3fcb3c6a4d2feb50248ab3ac26f3e05b Mon Sep 17 00:00:00 2001 From: Yu Watanabe Date: Tue, 2 Feb 2021 02:16:42 +0900 Subject: [PATCH 1/5] libudev: use hashmap_ensure_put() --- src/libudev/libudev-list.c | 8 +------- 1 file changed, 1 insertion(+), 7 deletions(-) diff --git a/src/libudev/libudev-list.c b/src/libudev/libudev-list.c index 3b2a2cdee4b..514927bc8fa 100644 --- a/src/libudev/libudev-list.c +++ b/src/libudev/libudev-list.c @@ -70,7 +70,6 @@ struct udev_list *udev_list_new(bool unique) { struct udev_list_entry *udev_list_entry_add(struct udev_list *list, const char *_name, const char *_value) { _cleanup_(udev_list_entry_freep) struct udev_list_entry *entry = NULL; _cleanup_free_ char *name = NULL, *value = NULL; - int r; assert(list); @@ -95,14 +94,9 @@ struct udev_list_entry *udev_list_entry_add(struct udev_list *list, const char * }; if (list->unique) { - r = hashmap_ensure_allocated(&list->unique_entries, &string_hash_ops); - if (r < 0) - return NULL; - udev_list_entry_free(hashmap_get(list->unique_entries, entry->name)); - r = hashmap_put(list->unique_entries, entry->name, entry); - if (r < 0) + if (hashmap_ensure_put(&list->unique_entries, &string_hash_ops, entry->name, entry) < 0) return NULL; list->uptodate = false; From 140716a51694ad2aa365869e3f8a741509a70959 Mon Sep 17 00:00:00 2001 From: Yu Watanabe Date: Tue, 2 Feb 2021 02:18:49 +0900 Subject: [PATCH 2/5] libudev: set entry->list after the entry is stored in the list This should not change anything. As hashmap_remove() is called before hashmap_ensure_put(). So, even if hashmap_ensure_put() fails, a wrong entry will not removed from the hashmap by udev_list_entry_free(). But anyway, just for safety. --- src/libudev/libudev-list.c | 3 ++- 1 file changed, 2 insertions(+), 1 deletion(-) diff --git a/src/libudev/libudev-list.c b/src/libudev/libudev-list.c index 514927bc8fa..c224e9df6d3 100644 --- a/src/libudev/libudev-list.c +++ b/src/libudev/libudev-list.c @@ -88,7 +88,6 @@ struct udev_list_entry *udev_list_entry_add(struct udev_list *list, const char * return NULL; *entry = (struct udev_list_entry) { - .list = list, .name = TAKE_PTR(name), .value = TAKE_PTR(value), }; @@ -103,6 +102,8 @@ struct udev_list_entry *udev_list_entry_add(struct udev_list *list, const char * } else LIST_APPEND(entries, list->entries, entry); + entry->list = list; + return TAKE_PTR(entry); } From 8e5ce38727246a213d88f43da72ae2769d99d3f6 Mon Sep 17 00:00:00 2001 From: Yu Watanabe Date: Tue, 2 Feb 2021 02:16:01 +0900 Subject: [PATCH 3/5] libudev: also drop the entry from LIST even if unique flag is set Otherwise, the list becomes dirty when an entry is freed. This also remove the entry from the hashmap only when its name is set. The name should be always set, so that does not change anything. But just for safety. --- src/libudev/libudev-list.c | 5 +++-- 1 file changed, 3 insertions(+), 2 deletions(-) diff --git a/src/libudev/libudev-list.c b/src/libudev/libudev-list.c index c224e9df6d3..d42008e48d6 100644 --- a/src/libudev/libudev-list.c +++ b/src/libudev/libudev-list.c @@ -39,9 +39,10 @@ static struct udev_list_entry *udev_list_entry_free(struct udev_list_entry *entr return NULL; if (entry->list) { - if (entry->list->unique) + if (entry->list->unique && entry->name) hashmap_remove(entry->list->unique_entries, entry->name); - else + + if (!entry->list->unique || entry->list->uptodate) LIST_REMOVE(entries, entry->list->entries, entry); } From 65c637ad2c2dc1fcddac01f104cdb146a88af852 Mon Sep 17 00:00:00 2001 From: Yu Watanabe Date: Tue, 2 Feb 2021 02:34:20 +0900 Subject: [PATCH 4/5] libudev: unset uptodate flag before free()ing entries udev_list_entry_free() also removes the entry from LIST if the flag is set. This slightly optimizes the cleanup logic. --- src/libudev/libudev-list.c | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/src/libudev/libudev-list.c b/src/libudev/libudev-list.c index d42008e48d6..d992d1879bf 100644 --- a/src/libudev/libudev-list.c +++ b/src/libudev/libudev-list.c @@ -115,8 +115,8 @@ void udev_list_cleanup(struct udev_list *list) { return; if (list->unique) { - hashmap_clear_with_destructor(list->unique_entries, udev_list_entry_free); list->uptodate = false; + hashmap_clear_with_destructor(list->unique_entries, udev_list_entry_free); } else LIST_FOREACH_SAFE(entries, i, n, list->entries) udev_list_entry_free(i); From ecf83c24295bbb7b940c91fb2aab387db74ef685 Mon Sep 17 00:00:00 2001 From: Yu Watanabe Date: Tue, 2 Feb 2021 03:23:31 +0900 Subject: [PATCH 5/5] libudev: add one more assertion --- src/libudev/libudev-list.c | 1 + 1 file changed, 1 insertion(+) diff --git a/src/libudev/libudev-list.c b/src/libudev/libudev-list.c index d992d1879bf..69efc1013c1 100644 --- a/src/libudev/libudev-list.c +++ b/src/libudev/libudev-list.c @@ -73,6 +73,7 @@ struct udev_list_entry *udev_list_entry_add(struct udev_list *list, const char * _cleanup_free_ char *name = NULL, *value = NULL; assert(list); + assert(_name); name = strdup(_name); if (!name)