From cf45350745ef02b80378761dcccf4e9744ef839a Mon Sep 17 00:00:00 2001 From: Yu Watanabe Date: Sat, 13 Mar 2021 15:57:55 +0900 Subject: [PATCH] ordered-set: make ordered_set_put_strdup() allocate OrderedSet object For consistency with set_put_strdup(). --- src/basic/ordered-set.c | 21 ++++++----- src/basic/ordered-set.h | 12 +++++-- src/network/networkd-network.c | 8 ++--- src/network/networkd-state-file.c | 58 +++++++++++-------------------- src/test/test-ordered-set.c | 27 +++++--------- 5 files changed, 53 insertions(+), 73 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-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); + 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); - q = ordered_set_new(&string_hash_ops); - assert_se(q); + 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(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(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_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")));