network/manager: hash_ops related cleanups (systemd#37121)

Author: yuwata

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) {

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");
 

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);

src/network/networkd-manager.c

@@ -672,15 +672,16 @@ 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_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. */
@@ -691,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);
 

src/network/networkd-network.c

@@ -44,6 +44,16 @@
 #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,
+                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 +115,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;
@@ -589,7 +599,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);
 
@@ -640,15 +650,15 @@ 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;
 
         assert(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);
@@ -670,14 +680,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)
@@ -688,11 +697,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) {
@@ -820,7 +824,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(network->ipv6_proxy_ndp_addresses);

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);
 }
 

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;
 

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);