From 919aeb666a33c79838bff4c6848d37c09b7f0562 Mon Sep 17 00:00:00 2001 From: Yu Watanabe Date: Sun, 13 Apr 2025 03:08:31 +0900 Subject: [PATCH 1/5] network/network: use hash_ops with destructor for managing stacked netdevs --- src/network/networkd-network.c | 11 ++++++++--- 1 file changed, 8 insertions(+), 3 deletions(-) diff --git a/src/network/networkd-network.c b/src/network/networkd-network.c index 00851243b5..e7a2e034a0 100644 --- a/src/network/networkd-network.c +++ b/src/network/networkd-network.c @@ -44,6 +44,11 @@ #include "strv.h" #include "tclass.h" +DEFINE_PRIVATE_HASH_OPS_WITH_VALUE_DESTRUCTOR( + stacked_netdevs_hash_ops, + char, string_hash_func, string_compare_func, + NetDev, netdev_unref); + static int network_resolve_netdev_one(Network *network, const char *name, NetDevKind kind, NetDev **ret) { const char *kind_string; NetDev *netdev; @@ -105,14 +110,14 @@ static int network_resolve_stacked_netdevs(Network *network) { if (network_resolve_netdev_one(network, name, PTR_TO_INT(kind), &netdev) <= 0) continue; - r = hashmap_ensure_put(&network->stacked_netdevs, &string_hash_ops, netdev->ifname, netdev); + r = hashmap_ensure_put(&network->stacked_netdevs, &stacked_netdevs_hash_ops, netdev->ifname, netdev); if (r == -ENOMEM) return log_oom(); if (r < 0) log_warning_errno(r, "%s: Failed to add NetDev '%s' to network, ignoring: %m", network->filename, (const char *) name); - netdev = NULL; + TAKE_PTR(netdev); } return 0; @@ -825,7 +830,7 @@ static Network *network_free(Network *network) { netdev_unref(network->bridge); netdev_unref(network->bond); netdev_unref(network->vrf); - hashmap_free_with_destructor(network->stacked_netdevs, netdev_unref); + hashmap_free(network->stacked_netdevs); /* static configs */ set_free_free(network->ipv6_proxy_ndp_addresses); From 09ddaf2af30a522cd8c585366a832d825aabac6e Mon Sep 17 00:00:00 2001 From: Yu Watanabe Date: Sun, 13 Apr 2025 03:17:38 +0900 Subject: [PATCH 2/5] hashmap: introduce ordered_hashmap_free_and_replace() --- src/basic/hashmap.h | 4 +++- 1 file changed, 3 insertions(+), 1 deletion(-) diff --git a/src/basic/hashmap.h b/src/basic/hashmap.h index 091062b5e9..c577bd6c22 100644 --- a/src/basic/hashmap.h +++ b/src/basic/hashmap.h @@ -88,8 +88,10 @@ OrderedHashmap* _ordered_hashmap_new(const struct hash_ops *hash_ops HASHMAP_DE #define hashmap_new(ops) _hashmap_new(ops HASHMAP_DEBUG_SRC_ARGS) #define ordered_hashmap_new(ops) _ordered_hashmap_new(ops HASHMAP_DEBUG_SRC_ARGS) -#define hashmap_free_and_replace(a, b) \ +#define hashmap_free_and_replace(a, b) \ free_and_replace_full(a, b, hashmap_free) +#define ordered_hashmap_free_and_replace(a, b) \ + free_and_replace_full(a, b, ordered_hashmap_free) HashmapBase* _hashmap_free(HashmapBase *h, free_func_t default_free_key, free_func_t default_free_value); static inline Hashmap* hashmap_free(Hashmap *h) { From a85f73fa55bf5848523287626f27fce54f763760 Mon Sep 17 00:00:00 2001 From: Yu Watanabe Date: Sun, 13 Apr 2025 03:18:12 +0900 Subject: [PATCH 3/5] network/network: use hash_ops with destructor for managing Network objects --- src/network/networkd-manager.c | 2 +- src/network/networkd-network.c | 21 ++++++++++----------- 2 files changed, 11 insertions(+), 12 deletions(-) diff --git a/src/network/networkd-manager.c b/src/network/networkd-manager.c index 128af9d0bf..0da3973d2b 100644 --- a/src/network/networkd-manager.c +++ b/src/network/networkd-manager.c @@ -680,7 +680,7 @@ Manager* manager_free(Manager *m) { m->links_by_index = hashmap_free_with_destructor(m->links_by_index, link_unref); m->dhcp_pd_subnet_ids = set_free(m->dhcp_pd_subnet_ids); - m->networks = ordered_hashmap_free_with_destructor(m->networks, network_unref); + m->networks = ordered_hashmap_free(m->networks); /* The same object may be registered with multiple names, and netdev_detach() may drop multiple * entries. Hence, hashmap_free_with_destructor() cannot be used. */ diff --git a/src/network/networkd-network.c b/src/network/networkd-network.c index e7a2e034a0..9994eeb769 100644 --- a/src/network/networkd-network.c +++ b/src/network/networkd-network.c @@ -44,6 +44,11 @@ #include "strv.h" #include "tclass.h" +DEFINE_PRIVATE_HASH_OPS_WITH_VALUE_DESTRUCTOR( + network_hash_ops, + char, string_hash_func, string_compare_func, + Network, network_unref); + DEFINE_PRIVATE_HASH_OPS_WITH_VALUE_DESTRUCTOR( stacked_netdevs_hash_ops, char, string_hash_func, string_compare_func, @@ -599,7 +604,7 @@ int network_load_one(Manager *manager, OrderedHashmap **networks, const char *fi if (r < 0) return r; /* network_verify() logs internally. */ - r = ordered_hashmap_ensure_put(networks, &string_hash_ops, network->name, network); + r = ordered_hashmap_ensure_put(networks, &network_hash_ops, network->name, network); if (r < 0) return log_warning_errno(r, "%s: Failed to store configuration into hashmap: %m", filename); @@ -650,7 +655,7 @@ static bool network_netdev_equal(Network *a, Network *b) { } int network_reload(Manager *manager) { - OrderedHashmap *new_networks = NULL; + _cleanup_ordered_hashmap_free_ OrderedHashmap *new_networks = NULL; Network *n, *old; int r; @@ -658,7 +663,7 @@ int network_reload(Manager *manager) { r = network_load(manager, &new_networks); if (r < 0) - goto failure; + return r; ORDERED_HASHMAP_FOREACH(n, new_networks) { r = network_get_by_name(manager, n->name, &old); @@ -680,14 +685,13 @@ int network_reload(Manager *manager) { /* Nothing updated, use the existing Network object, and drop the new one. */ r = ordered_hashmap_replace(new_networks, old->name, old); if (r < 0) - goto failure; + return r; network_ref(old); network_unref(n); } - ordered_hashmap_free_with_destructor(manager->networks, network_unref); - manager->networks = new_networks; + ordered_hashmap_free_and_replace(manager->networks, new_networks); r = manager_build_dhcp_pd_subnet_ids(manager); if (r < 0) @@ -698,11 +702,6 @@ int network_reload(Manager *manager) { return r; return 0; - -failure: - ordered_hashmap_free_with_destructor(new_networks, network_unref); - - return r; } int manager_build_dhcp_pd_subnet_ids(Manager *manager) { From 4cf443e6446c24f8a9b894695d868c428feb8b7f Mon Sep 17 00:00:00 2001 From: Yu Watanabe Date: Sun, 13 Apr 2025 03:30:44 +0900 Subject: [PATCH 4/5] network/link: use hash_ops with destructor for managing Link objects --- src/network/networkd-link.c | 13 +++++++++---- src/network/networkd-link.h | 2 ++ src/network/networkd-manager.c | 5 +++-- src/network/networkd-state-file.c | 9 +++------ 4 files changed, 17 insertions(+), 12 deletions(-) diff --git a/src/network/networkd-link.c b/src/network/networkd-link.c index 8a8c41eaac..c604b7bb71 100644 --- a/src/network/networkd-link.c +++ b/src/network/networkd-link.c @@ -261,7 +261,7 @@ static void link_free_engines(Link *link) { link->radv = sd_radv_unref(link->radv); } -static Link *link_free(Link *link) { +static Link* link_free(Link *link) { assert(link); (void) link_clear_sysctl_shadows(link); @@ -295,7 +295,7 @@ static Link *link_free(Link *link) { hashmap_free(link->bound_to_links); hashmap_free(link->bound_by_links); - set_free_with_destructor(link->slaves, link_unref); + set_free(link->slaves); network_unref(link->network); @@ -307,6 +307,11 @@ static Link *link_free(Link *link) { DEFINE_TRIVIAL_REF_UNREF_FUNC(Link, link, link_free); +DEFINE_HASH_OPS_WITH_VALUE_DESTRUCTOR( + link_hash_ops, + void, trivial_hash_func, trivial_compare_func, + Link, link_unref); + int link_get_by_index(Manager *m, int ifindex, Link **ret) { Link *link; @@ -985,7 +990,7 @@ static int link_append_to_master(Link *link) { if (link_get_master(link, &master) < 0) return 0; - r = set_ensure_put(&master->slaves, NULL, link); + r = set_ensure_put(&master->slaves, &link_hash_ops, link); if (r <= 0) return r; @@ -2746,7 +2751,7 @@ static int link_new(Manager *manager, sd_netlink_message *message, Link **ret) { .dns_over_tls_mode = _DNS_OVER_TLS_MODE_INVALID, }; - r = hashmap_ensure_put(&manager->links_by_index, NULL, INT_TO_PTR(link->ifindex), link); + r = hashmap_ensure_put(&manager->links_by_index, &link_hash_ops, INT_TO_PTR(link->ifindex), link); if (r < 0) return log_link_debug_errno(link, r, "Failed to store link into manager: %m"); diff --git a/src/network/networkd-link.h b/src/network/networkd-link.h index c5806cea33..b35f86e918 100644 --- a/src/network/networkd-link.h +++ b/src/network/networkd-link.h @@ -222,6 +222,8 @@ typedef struct Link { char **ntp; } Link; +extern const struct hash_ops link_hash_ops; + typedef int (*link_netlink_message_handler_t)(sd_netlink*, sd_netlink_message*, Link*); bool link_is_ready_to_configure(Link *link, bool allow_unmanaged); diff --git a/src/network/networkd-manager.c b/src/network/networkd-manager.c index 0da3973d2b..dbaebb1b0c 100644 --- a/src/network/networkd-manager.c +++ b/src/network/networkd-manager.c @@ -672,12 +672,13 @@ Manager* manager_free(Manager *m) { m->request_queue = ordered_set_free(m->request_queue); m->remove_request_queue = ordered_set_free(m->remove_request_queue); - m->dirty_links = set_free_with_destructor(m->dirty_links, link_unref); m->new_wlan_ifindices = set_free(m->new_wlan_ifindices); + + m->dirty_links = set_free(m->dirty_links); m->links_by_name = hashmap_free(m->links_by_name); m->links_by_hw_addr = hashmap_free(m->links_by_hw_addr); m->links_by_dhcp_pd_subnet_prefix = hashmap_free(m->links_by_dhcp_pd_subnet_prefix); - m->links_by_index = hashmap_free_with_destructor(m->links_by_index, link_unref); + m->links_by_index = hashmap_free(m->links_by_index); m->dhcp_pd_subnet_ids = set_free(m->dhcp_pd_subnet_ids); m->networks = ordered_hashmap_free(m->networks); diff --git a/src/network/networkd-state-file.c b/src/network/networkd-state-file.c index da917dd897..7c028dddbd 100644 --- a/src/network/networkd-state-file.c +++ b/src/network/networkd-state-file.c @@ -947,8 +947,6 @@ static int link_save(Link *link) { } void link_dirty(Link *link) { - int r; - assert(link); assert(link->manager); @@ -962,10 +960,9 @@ void link_dirty(Link *link) { /* Also mark manager dirty as link is dirty */ link->manager->dirty = true; - r = set_ensure_put(&link->manager->dirty_links, NULL, link); - if (r <= 0) - /* Ignore allocation errors and don't take another ref if the link was already dirty */ - return; + if (set_ensure_put(&link->manager->dirty_links, &link_hash_ops, link) <= 0) + return; /* Ignore allocation errors and don't take another ref if the link was already dirty */ + link_ref(link); } From 52278e06347dec58e3264c623e02ef72e510fc6d Mon Sep 17 00:00:00 2001 From: Yu Watanabe Date: Sun, 13 Apr 2025 03:38:50 +0900 Subject: [PATCH 5/5] network/wiphy: use hash_ops with destructor for managing Wiphy objects --- src/network/networkd-manager.c | 2 +- src/network/networkd-wiphy.c | 11 +++++++++-- src/network/networkd-wiphy.h | 3 --- 3 files changed, 10 insertions(+), 6 deletions(-) diff --git a/src/network/networkd-manager.c b/src/network/networkd-manager.c index dbaebb1b0c..0fa3ae2a5b 100644 --- a/src/network/networkd-manager.c +++ b/src/network/networkd-manager.c @@ -692,7 +692,7 @@ Manager* manager_free(Manager *m) { m->tuntap_fds_by_name = hashmap_free(m->tuntap_fds_by_name); m->wiphy_by_name = hashmap_free(m->wiphy_by_name); - m->wiphy_by_index = hashmap_free_with_destructor(m->wiphy_by_index, wiphy_free); + m->wiphy_by_index = hashmap_free(m->wiphy_by_index); ordered_set_free(m->address_pools); diff --git a/src/network/networkd-wiphy.c b/src/network/networkd-wiphy.c index 441713fa0c..798531f8e6 100644 --- a/src/network/networkd-wiphy.c +++ b/src/network/networkd-wiphy.c @@ -12,7 +12,7 @@ #include "udev-util.h" #include "wifi-util.h" -Wiphy *wiphy_free(Wiphy *w) { +static Wiphy* wiphy_free(Wiphy *w) { if (!w) return NULL; @@ -29,6 +29,13 @@ Wiphy *wiphy_free(Wiphy *w) { return mfree(w); } +DEFINE_TRIVIAL_CLEANUP_FUNC(Wiphy*, wiphy_free); + +DEFINE_PRIVATE_HASH_OPS_WITH_VALUE_DESTRUCTOR( + wiphy_hash_ops, + void, trivial_hash_func, trivial_compare_func, + Wiphy, wiphy_free); + static int wiphy_new(Manager *manager, sd_netlink_message *message, Wiphy **ret) { _cleanup_(wiphy_freep) Wiphy *w = NULL; _cleanup_free_ char *name = NULL; @@ -56,7 +63,7 @@ static int wiphy_new(Manager *manager, sd_netlink_message *message, Wiphy **ret) .name = TAKE_PTR(name), }; - r = hashmap_ensure_put(&manager->wiphy_by_index, NULL, UINT32_TO_PTR(w->index), w); + r = hashmap_ensure_put(&manager->wiphy_by_index, &wiphy_hash_ops, UINT32_TO_PTR(w->index), w); if (r < 0) return r; diff --git a/src/network/networkd-wiphy.h b/src/network/networkd-wiphy.h index b9056e802c..8f92112148 100644 --- a/src/network/networkd-wiphy.h +++ b/src/network/networkd-wiphy.h @@ -31,9 +31,6 @@ typedef struct Wiphy { RFKillState rfkill_state; } Wiphy; -Wiphy *wiphy_free(Wiphy *w); -DEFINE_TRIVIAL_CLEANUP_FUNC(Wiphy*, wiphy_free); - int wiphy_get_by_index(Manager *manager, uint32_t index, Wiphy **ret); int wiphy_get_by_name(Manager *manager, const char *name, Wiphy **ret);