diff --git a/src/libudev/libudev-list.c b/src/libudev/libudev-list.c index 0385c4679e..3ea77507c9 100644 --- a/src/libudev/libudev-list.c +++ b/src/libudev/libudev-list.c @@ -40,7 +40,7 @@ static struct udev_list_entry* udev_list_entry_free(struct udev_list_entry *entr if (entry->list) { if (entry->list->unique && entry->name) - hashmap_remove(entry->list->unique_entries, entry->name); + hashmap_remove_value(entry->list->unique_entries, entry->name, entry); if (!entry->list->unique || entry->list->uptodate) LIST_REMOVE(entries, entry->list->entries, entry); @@ -90,11 +90,19 @@ struct udev_list_entry* udev_list_entry_add(struct udev_list *list, const char * return NULL; if (list->unique) { - udev_list_entry_free(hashmap_get(list->unique_entries, entry->name)); + struct udev_list_entry *old; - if (hashmap_ensure_put(&list->unique_entries, &udev_list_entry_hash_ops, entry->name, entry) < 0) + /* Replace the entry for an already known name in a single step. hashmap_replace() reuses the + * existing bucket and hence never allocates, so an existing entry is never dropped, not even + * under memory pressure. Allocating may only fail when adding a genuinely new name, in which + * case there is nothing to lose. */ + old = hashmap_get(list->unique_entries, entry->name); + + if (hashmap_ensure_replace(&list->unique_entries, &udev_list_entry_hash_ops, entry->name, entry) < 0) return NULL; + udev_list_entry_free(old); + list->uptodate = false; } else LIST_APPEND(entries, list->entries, entry); diff --git a/src/udev/meson.build b/src/udev/meson.build index cbfca87203..893241db0f 100644 --- a/src/udev/meson.build +++ b/src/udev/meson.build @@ -203,7 +203,7 @@ executables += [ 'suite' : 'udev', }, udev_test_template + { - 'sources' : files('net/test-link-config-tables.c'), + 'sources' : files('net/test-link.c'), 'suite' : 'udev', }, udev_test_template + { diff --git a/src/udev/net/link-config.c b/src/udev/net/link-config.c index 9d2cb3730e..ec50a155ac 100644 --- a/src/udev/net/link-config.c +++ b/src/udev/net/link-config.c @@ -98,8 +98,7 @@ static void link_configs_free(LinkConfigContext *ctx) { ctx->stats_by_path = hashmap_free(ctx->stats_by_path); - LIST_FOREACH(configs, config, ctx->configs) - link_config_free(config); + LIST_CLEAR(configs, ctx->configs, link_config_free); } LinkConfigContext *link_config_ctx_free(LinkConfigContext *ctx) { diff --git a/src/udev/net/test-link-config-tables.c b/src/udev/net/test-link-config-tables.c deleted file mode 100644 index 58b628bc9b..0000000000 --- a/src/udev/net/test-link-config-tables.c +++ /dev/null @@ -1,14 +0,0 @@ -/* SPDX-License-Identifier: LGPL-2.1-or-later */ - -#include "link-config.h" -#include "test-tables.h" -#include "tests.h" - -int main(int argc, char **argv) { - test_setup_logging(LOG_DEBUG); - - test_table(MACAddressPolicy, mac_address_policy, MAC_ADDRESS_POLICY); - test_table(IRQAffinityPolicy, irq_affinity_policy, IRQ_AFFINITY_POLICY); - - return EXIT_SUCCESS; -} diff --git a/src/udev/net/test-link.c b/src/udev/net/test-link.c new file mode 100644 index 0000000000..624f51db37 --- /dev/null +++ b/src/udev/net/test-link.c @@ -0,0 +1,37 @@ +/* SPDX-License-Identifier: LGPL-2.1-or-later */ + +#include "fd-util.h" +#include "link-config.h" +#include "test-tables.h" +#include "tests.h" +#include "tmpfile-util.h" + +TEST(link_config_load_reload) { + /* Loading link configuration into an already-populated context (as link_config_load() itself + * does via its initial clear) must not leave ctx->configs pointing at freed entries. If the clear + * leaves the list head dangling, the reload below and the context teardown afterwards touch freed + * memory. */ + + _cleanup_(link_config_ctx_freep) LinkConfigContext *ctx = NULL; + _cleanup_(unlink_tempfilep) char filename[] = "/tmp/test-link-config.XXXXXX"; + _cleanup_fclose_ FILE *f = NULL; + + ASSERT_OK(link_config_ctx_new(&ctx)); + + ASSERT_OK(fmkostemp_safe(filename, "r+", &f)); + ASSERT_OK(fputs("[Match]\nOriginalName=*\n\n[Link]\nMTUBytes=1500\n", f)); + ASSERT_OK(fflush(f)); + + /* Populate the context with one configuration. */ + ASSERT_OK(link_load_one(ctx, filename)); + + /* Reload into the same, already-populated context. */ + ASSERT_OK(link_config_load(ctx)); +} + +TEST(tables) { + test_table(MACAddressPolicy, mac_address_policy, MAC_ADDRESS_POLICY); + test_table(IRQAffinityPolicy, irq_affinity_policy, IRQ_AFFINITY_POLICY); +} + +DEFINE_TEST_MAIN(LOG_DEBUG);