Merge pull request #18989 from yuwata/ordered-set-put-strdup

ordered-set: make ordered_set_put_strdup() allocate OrderedSet object

7 files changed, 62 insertions, 82 deletions

diff --git a/src/basic/ordered-set.c b/src/basic/ordered-set.c
index 58fa8af1b7..0a76f04561 100644
--- a/src/basic/ordered-set.c
+++ b/src/basic/ordered-set.c
@@ -35,30 +35,33 @@ int ordered_set_consume(OrderedSet *s, void *p) {
         return r;
 }
 
-int ordered_set_put_strdup(OrderedSet *s, const char *p) {
+int _ordered_set_put_strdup(OrderedSet **s, const char *p  HASHMAP_DEBUG_PARAMS) {
         char *c;
         int r;
 
         assert(s);
         assert(p);
 
+        r = _ordered_set_ensure_allocated(s, &string_hash_ops_free  HASHMAP_DEBUG_PASS_ARGS);
+        if (r < 0)
+                return r;
+
+        if (ordered_set_contains(*s, p))
+                return 0;
+
         c = strdup(p);
         if (!c)
                 return -ENOMEM;
 
-        r = ordered_set_consume(s, c);
-        if (r == -EEXIST)
-                return 0;
-
-        return r;
+        return ordered_set_consume(*s, c);
 }
 
-int ordered_set_put_strdupv(OrderedSet *s, char **l) {
+int _ordered_set_put_strdupv(OrderedSet **s, char **l  HASHMAP_DEBUG_PARAMS) {
         int n = 0, r;
         char **i;
 
         STRV_FOREACH(i, l) {
-                r = ordered_set_put_strdup(s, *i);
+                r = _ordered_set_put_strdup(s, *i  HASHMAP_DEBUG_PASS_ARGS);
                 if (r < 0)
                         return r;
 
@@ -68,7 +71,7 @@ int ordered_set_put_strdupv(OrderedSet *s, char **l) {
         return n;
 }
 
-int ordered_set_put_string_set(OrderedSet *s, OrderedSet *l) {
+int ordered_set_put_string_set(OrderedSet **s, OrderedSet *l) {
         int n = 0, r;
         char *p;
 
diff --git a/src/basic/ordered-set.h b/src/basic/ordered-set.h
index baf8202088..a377f20b1f 100644
--- a/src/basic/ordered-set.h
+++ b/src/basic/ordered-set.h
@@ -26,6 +26,10 @@ static inline OrderedSet* ordered_set_free_free(OrderedSet *s) {
         return (OrderedSet*) ordered_hashmap_free_free((OrderedHashmap*) s);
 }
 
+static inline int ordered_set_contains(OrderedSet *s, const void *p) {
+        return ordered_hashmap_contains((OrderedHashmap*) s, p);
+}
+
 static inline int ordered_set_put(OrderedSet *s, void *p) {
         return ordered_hashmap_put((OrderedHashmap*) s, p, p);
 }
@@ -59,9 +63,11 @@ static inline char** ordered_set_get_strv(OrderedSet *s) {
 }
 
 int ordered_set_consume(OrderedSet *s, void *p);
-int ordered_set_put_strdup(OrderedSet *s, const char *p);
-int ordered_set_put_strdupv(OrderedSet *s, char **l);
-int ordered_set_put_string_set(OrderedSet *s, OrderedSet *l);
+int _ordered_set_put_strdup(OrderedSet **s, const char *p  HASHMAP_DEBUG_PARAMS);
+#define ordered_set_put_strdup(s, p) _ordered_set_put_strdup(s, p  HASHMAP_DEBUG_SRC_ARGS)
+int _ordered_set_put_strdupv(OrderedSet **s, char **l  HASHMAP_DEBUG_PARAMS);
+#define ordered_set_put_strdupv(s, l) _ordered_set_put_strdupv(s, l  HASHMAP_DEBUG_SRC_ARGS)
+int ordered_set_put_string_set(OrderedSet **s, OrderedSet *l);
 void ordered_set_print(FILE *f, const char *field, OrderedSet *s);
 
 #define _ORDERED_SET_FOREACH(e, s, i) \
diff --git a/src/network/networkd-link-bus.c b/src/network/networkd-link-bus.c
index f57828b57d..4d0e4815db 100644
--- a/src/network/networkd-link-bus.c
+++ b/src/network/networkd-link-bus.c
@@ -178,7 +178,7 @@ int bus_link_method_set_dns_servers_ex(sd_bus_message *message, void *userdata,
 }
 
 int bus_link_method_set_domains(sd_bus_message *message, void *userdata, sd_bus_error *error) {
-        _cleanup_(ordered_set_freep) OrderedSet *search_domains = NULL, *route_domains = NULL;
+        _cleanup_ordered_set_free_ OrderedSet *search_domains = NULL, *route_domains = NULL;
         Link *l = userdata;
         int r;
 
@@ -218,15 +218,15 @@ int bus_link_method_set_domains(sd_bus_message *message, void *userdata, sd_bus_
                         return sd_bus_error_setf(error, SD_BUS_ERROR_INVALID_ARGS, "Invalid search domain %s", name);
 
                 domains = route_only ? &route_domains : &search_domains;
-                r = ordered_set_ensure_allocated(domains, &string_hash_ops);
+                r = ordered_set_ensure_allocated(domains, &string_hash_ops_free);
                 if (r < 0)
                         return r;
 
-                r = ordered_set_put(*domains, str);
+                r = ordered_set_consume(*domains, TAKE_PTR(str));
+                if (r == -EEXIST)
+                        continue;
                 if (r < 0)
                         return r;
-
-                TAKE_PTR(str);
         }
 
         r = sd_bus_message_exit_container(message);
@@ -242,8 +242,8 @@ int bus_link_method_set_domains(sd_bus_message *message, void *userdata, sd_bus_
         if (r == 0)
                 return 1; /* Polkit will call us back */
 
-        ordered_set_free_free(l->search_domains);
-        ordered_set_free_free(l->route_domains);
+        ordered_set_free(l->search_domains);
+        ordered_set_free(l->route_domains);
         l->search_domains = TAKE_PTR(search_domains);
         l->route_domains = TAKE_PTR(route_domains);
 
diff --git a/src/network/networkd-link.c b/src/network/networkd-link.c
index 6868a7ccb2..de453fa72e 100644
--- a/src/network/networkd-link.c
+++ b/src/network/networkd-link.c
@@ -478,8 +478,8 @@ void link_dns_settings_clear(Link *link) {
         link->dns = mfree(link->dns);
         link->n_dns = UINT_MAX;
 
-        link->search_domains = ordered_set_free_free(link->search_domains);
-        link->route_domains = ordered_set_free_free(link->route_domains);
+        link->search_domains = ordered_set_free(link->search_domains);
+        link->route_domains = ordered_set_free(link->route_domains);
 
         link->dns_default_route = -1;
         link->llmnr = _RESOLVE_SUPPORT_INVALID;
diff --git a/src/network/networkd-network.c b/src/network/networkd-network.c
index e5ffd35b6f..f532536f1c 100644
--- a/src/network/networkd-network.c
+++ b/src/network/networkd-network.c
@@ -896,11 +896,9 @@ int config_parse_domains(
                 }
 
                 OrderedSet **set = is_route ? &n->route_domains : &n->search_domains;
-                r = ordered_set_ensure_allocated(set, &string_hash_ops_free);
-                if (r < 0)
-                        return log_oom();
-
-                r = ordered_set_put_strdup(*set, domain);
+                r = ordered_set_put_strdup(set, domain);
+                if (r == -EEXIST)
+                        continue;
                 if (r < 0)
                         return log_oom();
         }
diff --git a/src/network/networkd-state-file.c b/src/network/networkd-state-file.c
index 9c5c5047b3..e25a1497ec 100644
--- a/src/network/networkd-state-file.c
+++ b/src/network/networkd-state-file.c
@@ -19,7 +19,7 @@
 #include "strv.h"
 #include "tmpfile-util.h"
 
-static int ordered_set_put_dns_server(OrderedSet *s, int ifindex, struct in_addr_full *dns) {
+static int ordered_set_put_dns_server(OrderedSet **s, int ifindex, struct in_addr_full *dns) {
         const char *p;
         int r;
 
@@ -40,7 +40,7 @@ static int ordered_set_put_dns_server(OrderedSet *s, int ifindex, struct in_addr
         return r;
 }
 
-static int ordered_set_put_dns_servers(OrderedSet *s, int ifindex, struct in_addr_full **dns, unsigned n) {
+static int ordered_set_put_dns_servers(OrderedSet **s, int ifindex, struct in_addr_full **dns, unsigned n) {
         int r, c = 0;
 
         assert(s);
@@ -57,8 +57,8 @@ static int ordered_set_put_dns_servers(OrderedSet *s, int ifindex, struct in_add
         return c;
 }
 
-static int ordered_set_put_in4_addr(OrderedSet *s, const struct in_addr *address) {
-        char *p;
+static int ordered_set_put_in4_addr(OrderedSet **s, const struct in_addr *address) {
+        _cleanup_free_ char *p = NULL;
         int r;
 
         assert(s);
@@ -68,7 +68,11 @@ static int ordered_set_put_in4_addr(OrderedSet *s, const struct in_addr *address
         if (r < 0)
                 return r;
 
-        r = ordered_set_consume(s, p);
+        r = ordered_set_ensure_allocated(s, &string_hash_ops_free);
+        if (r < 0)
+                return r;
+
+        r = ordered_set_consume(*s, TAKE_PTR(p));
         if (r == -EEXIST)
                 return 0;
 
@@ -76,7 +80,7 @@ static int ordered_set_put_in4_addr(OrderedSet *s, const struct in_addr *address
 }
 
 static int ordered_set_put_in4_addrv(
-                OrderedSet *s,
+                OrderedSet **s,
                 const struct in_addr *addresses,
                 size_t n,
                 bool (*predicate)(const struct in_addr *addr)) {
@@ -100,7 +104,7 @@ static int ordered_set_put_in4_addrv(
 }
 
 int manager_save(Manager *m) {
-        _cleanup_ordered_set_free_free_ OrderedSet *dns = NULL, *ntp = NULL, *sip = NULL, *search_domains = NULL, *route_domains = NULL;
+        _cleanup_ordered_set_free_ OrderedSet *dns = NULL, *ntp = NULL, *sip = NULL, *search_domains = NULL, *route_domains = NULL;
         const char *operstate_str, *carrier_state_str, *address_state_str;
         LinkOperationalState operstate = LINK_OPERSTATE_OFF;
         LinkCarrierState carrier_state = LINK_CARRIER_STATE_OFF;
@@ -114,27 +118,6 @@ int manager_save(Manager *m) {
         assert(m);
         assert(m->state_file);
 
-        /* We add all NTP and DNS server to a set, to filter out duplicates */
-        dns = ordered_set_new(&string_hash_ops);
-        if (!dns)
-                return -ENOMEM;
-
-        ntp = ordered_set_new(&string_hash_ops);
-        if (!ntp)
-                return -ENOMEM;
-
-        sip = ordered_set_new(&string_hash_ops);
-        if (!sip)
-                return -ENOMEM;
-
-        search_domains = ordered_set_new(&dns_name_hash_ops);
-        if (!search_domains)
-                return -ENOMEM;
-
-        route_domains = ordered_set_new(&dns_name_hash_ops);
-        if (!route_domains)
-                return -ENOMEM;
-
         HASHMAP_FOREACH(link, m->links) {
                 const struct in_addr *addresses;
 
@@ -155,21 +138,21 @@ int manager_save(Manager *m) {
 
                 /* First add the static configured entries */
                 if (link->n_dns != UINT_MAX)
-                        r = ordered_set_put_dns_servers(dns, link->ifindex, link->dns, link->n_dns);
+                        r = ordered_set_put_dns_servers(&dns, link->ifindex, link->dns, link->n_dns);
                 else
-                        r = ordered_set_put_dns_servers(dns, link->ifindex, link->network->dns, link->network->n_dns);
+                        r = ordered_set_put_dns_servers(&dns, link->ifindex, link->network->dns, link->network->n_dns);
                 if (r < 0)
                         return r;
 
-                r = ordered_set_put_strdupv(ntp, link->ntp ?: link->network->ntp);
+                r = ordered_set_put_strdupv(&ntp, link->ntp ?: link->network->ntp);
                 if (r < 0)
                         return r;
 
-                r = ordered_set_put_string_set(search_domains, link->search_domains ?: link->network->search_domains);
+                r = ordered_set_put_string_set(&search_domains, link->search_domains ?: link->network->search_domains);
                 if (r < 0)
                         return r;
 
-                r = ordered_set_put_string_set(route_domains, link->route_domains ?: link->network->route_domains);
+                r = ordered_set_put_string_set(&route_domains, link->route_domains ?: link->network->route_domains);
                 if (r < 0)
                         return r;
 
@@ -180,7 +163,7 @@ int manager_save(Manager *m) {
                 if (link->network->dhcp_use_dns) {
                         r = sd_dhcp_lease_get_dns(link->dhcp_lease, &addresses);
                         if (r > 0) {
-                                r = ordered_set_put_in4_addrv(dns, addresses, r, in4_addr_is_non_local);
+                                r = ordered_set_put_in4_addrv(&dns, addresses, r, in4_addr_is_non_local);
                                 if (r < 0)
                                         return r;
                         } else if (r < 0 && r != -ENODATA)
@@ -190,7 +173,7 @@ int manager_save(Manager *m) {
                 if (link->network->dhcp_use_ntp) {
                         r = sd_dhcp_lease_get_ntp(link->dhcp_lease, &addresses);
                         if (r > 0) {
-                                r = ordered_set_put_in4_addrv(ntp, addresses, r, in4_addr_is_non_local);
+                                r = ordered_set_put_in4_addrv(&ntp, addresses, r, in4_addr_is_non_local);
                                 if (r < 0)
                                         return r;
                         } else if (r < 0 && r != -ENODATA)
@@ -200,7 +183,7 @@ int manager_save(Manager *m) {
                 if (link->network->dhcp_use_sip) {
                         r = sd_dhcp_lease_get_sip(link->dhcp_lease, &addresses);
                         if (r > 0) {
-                                r = ordered_set_put_in4_addrv(sip, addresses, r, in4_addr_is_non_local);
+                                r = ordered_set_put_in4_addrv(&sip, addresses, r, in4_addr_is_non_local);
                                 if (r < 0)
                                         return r;
                         } else if (r < 0 && r != -ENODATA)
@@ -208,10 +191,11 @@ int manager_save(Manager *m) {
                 }
 
                 if (link->network->dhcp_use_domains != DHCP_USE_DOMAINS_NO) {
+                        OrderedSet **target_domains;
                         const char *domainname;
                         char **domains = NULL;
 
-                        OrderedSet *target_domains = (link->network->dhcp_use_domains == DHCP_USE_DOMAINS_YES) ? search_domains : route_domains;
+                        target_domains = (link->network->dhcp_use_domains == DHCP_USE_DOMAINS_YES) ? &search_domains : &route_domains;
                         r = sd_dhcp_lease_get_domainname(link->dhcp_lease, &domainname);
                         if (r >= 0) {
                                 r = ordered_set_put_strdup(target_domains, domainname);
diff --git a/src/test/test-ordered-set.c b/src/test/test-ordered-set.c
index 0fbdd9706f..8d3c5b0d8c 100644
--- a/src/test/test-ordered-set.c
+++ b/src/test/test-ordered-set.c
@@ -95,31 +95,20 @@ static void test_set_put(void) {
 }
 
 static void test_set_put_string_set(void) {
-        _cleanup_ordered_set_free_free_ OrderedSet *m = NULL;
-        _cleanup_ordered_set_free_ OrderedSet *q = NULL;
+        _cleanup_ordered_set_free_ OrderedSet *m = NULL, *q = NULL;
         _cleanup_free_ char **final = NULL; /* "just free" because the strings are in the set */
-        void *t;
 
         log_info("/* %s */", __func__);
 
-        m = ordered_set_new(&string_hash_ops);
-        assert_se(m);
-
-        q = ordered_set_new(&string_hash_ops);
-        assert_se(q);
-
-        assert_se(t = strdup("1"));
-        assert_se(ordered_set_put(m, t) == 1);
-        assert_se(t = strdup("22"));
-        assert_se(ordered_set_put(m, t) == 1);
-        assert_se(t = strdup("333"));
-        assert_se(ordered_set_put(m, t) == 1);
+        assert_se(ordered_set_put_strdup(&m, "1") == 1);
+        assert_se(ordered_set_put_strdup(&m, "22") == 1);
+        assert_se(ordered_set_put_strdup(&m, "333") == 1);
 
-        assert_se(ordered_set_put(q, (void*) "11") == 1);
-        assert_se(ordered_set_put(q, (void*) "22") == 1);
-        assert_se(ordered_set_put(q, (void*) "33") == 1);
+        assert_se(ordered_set_put_strdup(&q, "11") == 1);
+        assert_se(ordered_set_put_strdup(&q, "22") == 1);
+        assert_se(ordered_set_put_strdup(&q, "33") == 1);
 
-        assert_se(ordered_set_put_string_set(m, q) == 2);
+        assert_se(ordered_set_put_string_set(&m, q) == 2);
 
         assert_se(final = ordered_set_get_strv(m));
         assert_se(strv_equal(final, STRV_MAKE("1", "22", "333", "11", "33")));