From ee05335f43698fc89e0dc733992c6e94b64bc8f3 Mon Sep 17 00:00:00 2001 From: Yu Watanabe Date: Sun, 25 Nov 2018 22:01:10 +0900 Subject: [PATCH 01/21] hashmap: fix minor coding style issue --- src/basic/hashmap.c | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/src/basic/hashmap.c b/src/basic/hashmap.c index eba56add1f..a59640023f 100644 --- a/src/basic/hashmap.c +++ b/src/basic/hashmap.c @@ -848,7 +848,7 @@ int internal_set_ensure_allocated(Set **s, const struct hash_ops *hash_ops HASH static void hashmap_free_no_clear(HashmapBase *h) { assert(!h->has_indirect); - assert(!h->n_direct_entries); + assert(h->n_direct_entries == 0); #if ENABLE_DEBUG_HASHMAP assert_se(pthread_mutex_lock(&hashmap_debug_list_mutex) == 0); From e30f9c972b789152d67ff34fd3bda294d20d1f51 Mon Sep 17 00:00:00 2001 From: Yu Watanabe Date: Tue, 27 Nov 2018 16:33:28 +0900 Subject: [PATCH 02/21] util: define free_func_t --- src/basic/alloc-util.h | 2 ++ src/basic/static-destruct.h | 3 ++- 2 files changed, 4 insertions(+), 1 deletion(-) diff --git a/src/basic/alloc-util.h b/src/basic/alloc-util.h index 2a6deb12ca..7d237720b9 100644 --- a/src/basic/alloc-util.h +++ b/src/basic/alloc-util.h @@ -8,6 +8,8 @@ #include "macro.h" +typedef void (*free_func_t)(void *p); + #define new(t, n) ((t*) malloc_multiply(sizeof(t), (n))) #define new0(t, n) ((t*) calloc((n), sizeof(t))) diff --git a/src/basic/static-destruct.h b/src/basic/static-destruct.h index cad838083f..4a9a40a835 100644 --- a/src/basic/static-destruct.h +++ b/src/basic/static-destruct.h @@ -1,5 +1,6 @@ #pragma once +#include "alloc-util.h" #include "macro.h" /* A framework for registering static variables that shall be freed on shutdown of a process. It's a bit like gcc's @@ -9,7 +10,7 @@ typedef struct StaticDestructor { void *data; - void (*destroy)(void *p); + free_func_t destroy; } StaticDestructor; #define STATIC_DESTRUCTOR_REGISTER(variable, func) \ From d1005d1c0050d3dc3a24c054bac4c4916073cbba Mon Sep 17 00:00:00 2001 From: Yu Watanabe Date: Wed, 28 Nov 2018 14:10:04 +0900 Subject: [PATCH 03/21] hash-funcs: introduce macro to create typesafe hash_ops --- src/basic/hash-funcs.h | 14 ++++++++++++++ 1 file changed, 14 insertions(+) diff --git a/src/basic/hash-funcs.h b/src/basic/hash-funcs.h index fa45cfe256..30e8979fe6 100644 --- a/src/basic/hash-funcs.h +++ b/src/basic/hash-funcs.h @@ -12,6 +12,20 @@ struct hash_ops { compare_func_t compare; }; +#define _DEFINE_HASH_OPS(uq, name, type, hash_func, compare_func, scope) \ + _unused_ static void (* UNIQ_T(static_hash_wrapper, uq))(const type *, struct siphash *) = hash_func; \ + _unused_ static int (* UNIQ_T(static_compare_wrapper, uq))(const type *, const type *) = compare_func; \ + scope const struct hash_ops name = { \ + .hash = (hash_func_t) hash_func, \ + .compare = (compare_func_t) compare_func, \ + } + +#define DEFINE_HASH_OPS(name, type, hash_func, compare_func) \ + _DEFINE_HASH_OPS(UNIQ, name, type, hash_func, compare_func,) + +#define DEFINE_PRIVATE_HASH_OPS(name, type, hash_func, compare_func) \ + _DEFINE_HASH_OPS(UNIQ, name, type, hash_func, compare_func, static) + void string_hash_func(const void *p, struct siphash *state); int string_compare_func(const void *a, const void *b) _pure_; extern const struct hash_ops string_hash_ops; From 25073e5012cdb4de13d815197815c33194ff7dc9 Mon Sep 17 00:00:00 2001 From: Yu Watanabe Date: Sun, 2 Dec 2018 07:46:33 +0100 Subject: [PATCH 04/21] hash-funcs: make basic hash_ops typesafe --- src/basic/hash-funcs.c | 49 +++++++++++------------------------------- src/basic/hash-funcs.h | 16 +++++++------- 2 files changed, 21 insertions(+), 44 deletions(-) diff --git a/src/basic/hash-funcs.c b/src/basic/hash-funcs.c index 6f6832d237..d6dc34e60c 100644 --- a/src/basic/hash-funcs.c +++ b/src/basic/hash-funcs.c @@ -5,21 +5,13 @@ #include "hash-funcs.h" #include "path-util.h" -void string_hash_func(const void *p, struct siphash *state) { +void string_hash_func(const char *p, struct siphash *state) { siphash24_compress(p, strlen(p) + 1, state); } -int string_compare_func(const void *a, const void *b) { - return strcmp(a, b); -} +DEFINE_HASH_OPS(string_hash_ops, char, string_hash_func, string_compare_func); -const struct hash_ops string_hash_ops = { - .hash = string_hash_func, - .compare = string_compare_func -}; - -void path_hash_func(const void *p, struct siphash *state) { - const char *q = p; +void path_hash_func(const char *q, struct siphash *state) { size_t n; assert(q); @@ -57,14 +49,11 @@ void path_hash_func(const void *p, struct siphash *state) { } } -int path_compare_func(const void *a, const void *b) { +int path_compare_func(const char *a, const char *b) { return path_compare(a, b); } -const struct hash_ops path_hash_ops = { - .hash = path_hash_func, - .compare = path_compare_func -}; +DEFINE_HASH_OPS(path_hash_ops, char, path_hash_func, path_compare_func); void trivial_hash_func(const void *p, struct siphash *state) { siphash24_compress(&p, sizeof(p), state); @@ -79,36 +68,24 @@ const struct hash_ops trivial_hash_ops = { .compare = trivial_compare_func }; -void uint64_hash_func(const void *p, struct siphash *state) { +void uint64_hash_func(const uint64_t *p, struct siphash *state) { siphash24_compress(p, sizeof(uint64_t), state); } -int uint64_compare_func(const void *_a, const void *_b) { - uint64_t a, b; - a = *(const uint64_t*) _a; - b = *(const uint64_t*) _b; - return CMP(a, b); +int uint64_compare_func(const uint64_t *a, const uint64_t *b) { + return CMP(*a, *b); } -const struct hash_ops uint64_hash_ops = { - .hash = uint64_hash_func, - .compare = uint64_compare_func -}; +DEFINE_HASH_OPS(uint64_hash_ops, uint64_t, uint64_hash_func, uint64_compare_func); #if SIZEOF_DEV_T != 8 -void devt_hash_func(const void *p, struct siphash *state) { +void devt_hash_func(const dev_t *p, struct siphash *state) { siphash24_compress(p, sizeof(dev_t), state); } -int devt_compare_func(const void *_a, const void *_b) { - dev_t a, b; - a = *(const dev_t*) _a; - b = *(const dev_t*) _b; - return CMP(a, b); +int devt_compare_func(const dev_t *a, const dev_t *b) { + return CMP(*a, *b); } -const struct hash_ops devt_hash_ops = { - .hash = devt_hash_func, - .compare = devt_compare_func -}; +DEFINE_HASH_OPS(devt_hash_ops, dev_t, devt_hash_func, devt_compare_func); #endif diff --git a/src/basic/hash-funcs.h b/src/basic/hash-funcs.h index 30e8979fe6..61c36a2119 100644 --- a/src/basic/hash-funcs.h +++ b/src/basic/hash-funcs.h @@ -26,12 +26,12 @@ struct hash_ops { #define DEFINE_PRIVATE_HASH_OPS(name, type, hash_func, compare_func) \ _DEFINE_HASH_OPS(UNIQ, name, type, hash_func, compare_func, static) -void string_hash_func(const void *p, struct siphash *state); -int string_compare_func(const void *a, const void *b) _pure_; +void string_hash_func(const char *p, struct siphash *state); +#define string_compare_func strcmp extern const struct hash_ops string_hash_ops; -void path_hash_func(const void *p, struct siphash *state); -int path_compare_func(const void *a, const void *b) _pure_; +void path_hash_func(const char *p, struct siphash *state); +int path_compare_func(const char *a, const char *b) _pure_; extern const struct hash_ops path_hash_ops; /* This will compare the passed pointers directly, and will not dereference them. This is hence not useful for strings @@ -42,15 +42,15 @@ extern const struct hash_ops trivial_hash_ops; /* 32bit values we can always just embed in the pointer itself, but in order to support 32bit archs we need store 64bit * values indirectly, since they don't fit in a pointer. */ -void uint64_hash_func(const void *p, struct siphash *state); -int uint64_compare_func(const void *a, const void *b) _pure_; +void uint64_hash_func(const uint64_t *p, struct siphash *state); +int uint64_compare_func(const uint64_t *a, const uint64_t *b) _pure_; extern const struct hash_ops uint64_hash_ops; /* On some archs dev_t is 32bit, and on others 64bit. And sometimes it's 64bit on 32bit archs, and sometimes 32bit on * 64bit archs. Yuck! */ #if SIZEOF_DEV_T != 8 -void devt_hash_func(const void *p, struct siphash *state) _pure_; -int devt_compare_func(const void *a, const void *b) _pure_; +void devt_hash_func(const dev_t *p, struct siphash *state) _pure_; +int devt_compare_func(const dev_t *a, const dev_t *b) _pure_; extern const struct hash_ops devt_hash_ops; #else #define devt_hash_func uint64_hash_func From 7a08d314f20b5d6ca44c573426a8a5c812020344 Mon Sep 17 00:00:00 2001 From: Yu Watanabe Date: Tue, 27 Nov 2018 22:25:20 +0900 Subject: [PATCH 05/21] tree-wide: make hash_ops typesafe --- src/basic/ether-addr-util.c | 12 ++----- src/basic/ether-addr-util.h | 2 +- src/basic/in-addr-util.c | 12 ++----- src/basic/in-addr-util.h | 2 -- src/busctl/busctl.c | 10 ++---- src/core/socket.c | 12 ++----- src/journal/catalog.c | 9 ++---- src/libsystemd-network/dhcp-server-internal.h | 4 +-- src/libsystemd-network/lldp-neighbor.c | 9 ++---- src/libsystemd-network/sd-dhcp-server.c | 15 ++------- src/libsystemd/sd-bus/bus-objects.c | 12 ++----- src/libsystemd/sd-event/sd-event.c | 14 +++----- src/libsystemd/sd-id128/id128-util.c | 11 +++---- src/libsystemd/sd-id128/id128-util.h | 4 +-- src/network/networkd-address.c | 12 ++----- src/network/networkd-manager.c | 15 +++------ src/network/networkd-ndisc.c | 26 ++++----------- src/network/networkd-network.c | 12 ++----- src/network/networkd-route.c | 12 ++----- src/network/networkd-routing-policy-rule.c | 12 ++----- src/resolve/resolved-dns-packet.c | 12 ++----- src/resolve/resolved-dns-rr.c | 32 ++++++------------- src/resolve/resolved-dns-rr.h | 2 +- src/resolve/resolved-dns-server.c | 12 ++----- src/shared/dns-domain.c | 14 +++----- src/shared/dns-domain.h | 4 +-- src/test/test-prioq.c | 15 +++------ 27 files changed, 85 insertions(+), 223 deletions(-) diff --git a/src/basic/ether-addr-util.c b/src/basic/ether-addr-util.c index 461ace73e2..e875696a1a 100644 --- a/src/basic/ether-addr-util.c +++ b/src/basic/ether-addr-util.c @@ -28,21 +28,15 @@ char* ether_addr_to_string(const struct ether_addr *addr, char buffer[ETHER_ADDR return buffer; } -int ether_addr_compare(const void *a, const void *b) { - assert(a); - assert(b); - +int ether_addr_compare(const struct ether_addr *a, const struct ether_addr *b) { return memcmp(a, b, ETH_ALEN); } -static void ether_addr_hash_func(const void *p, struct siphash *state) { +static void ether_addr_hash_func(const struct ether_addr *p, struct siphash *state) { siphash24_compress(p, sizeof(struct ether_addr), state); } -const struct hash_ops ether_addr_hash_ops = { - .hash = ether_addr_hash_func, - .compare = ether_addr_compare -}; +DEFINE_HASH_OPS(ether_addr_hash_ops, struct ether_addr, ether_addr_hash_func, ether_addr_compare); int ether_addr_from_string(const char *s, struct ether_addr *ret) { size_t pos = 0, n, field; diff --git a/src/basic/ether-addr-util.h b/src/basic/ether-addr-util.h index 3be0370049..4e44b30be9 100644 --- a/src/basic/ether-addr-util.h +++ b/src/basic/ether-addr-util.h @@ -12,7 +12,7 @@ #define ETHER_ADDR_TO_STRING_MAX (3*6) char* ether_addr_to_string(const struct ether_addr *addr, char buffer[ETHER_ADDR_TO_STRING_MAX]); -int ether_addr_compare(const void *a, const void *b); +int ether_addr_compare(const struct ether_addr *a, const struct ether_addr *b); static inline bool ether_addr_equal(const struct ether_addr *a, const struct ether_addr *b) { return ether_addr_compare(a, b) == 0; } diff --git a/src/basic/in-addr-util.c b/src/basic/in-addr-util.c index 6a00de2ef0..4bb7d9c5e0 100644 --- a/src/basic/in-addr-util.c +++ b/src/basic/in-addr-util.c @@ -603,15 +603,12 @@ int in_addr_prefix_from_string_auto_internal( } -void in_addr_data_hash_func(const void *p, struct siphash *state) { - const struct in_addr_data *a = p; - +static void in_addr_data_hash_func(const struct in_addr_data *a, struct siphash *state) { siphash24_compress(&a->family, sizeof(a->family), state); siphash24_compress(&a->address, FAMILY_ADDRESS_SIZE(a->family), state); } -int in_addr_data_compare_func(const void *a, const void *b) { - const struct in_addr_data *x = a, *y = b; +static int in_addr_data_compare_func(const struct in_addr_data *x, const struct in_addr_data *y) { int r; r = CMP(x->family, y->family); @@ -621,7 +618,4 @@ int in_addr_data_compare_func(const void *a, const void *b) { return memcmp(&x->address, &y->address, FAMILY_ADDRESS_SIZE(x->family)); } -const struct hash_ops in_addr_data_hash_ops = { - .hash = in_addr_data_hash_func, - .compare = in_addr_data_compare_func, -}; +DEFINE_HASH_OPS(in_addr_data_hash_ops, struct in_addr_data, in_addr_data_hash_func, in_addr_data_compare_func); diff --git a/src/basic/in-addr-util.h b/src/basic/in-addr-util.h index 2f64c31f96..5de85cc422 100644 --- a/src/basic/in-addr-util.h +++ b/src/basic/in-addr-util.h @@ -69,6 +69,4 @@ static inline size_t FAMILY_ADDRESS_SIZE(int family) { * See also oss-fuzz#11344. */ #define IN_ADDR_NULL ((union in_addr_union) { .in6 = {} }) -void in_addr_data_hash_func(const void *p, struct siphash *state); -int in_addr_data_compare_func(const void *a, const void *b); extern const struct hash_ops in_addr_data_hash_ops; diff --git a/src/busctl/busctl.c b/src/busctl/busctl.c index 1b0939caba..6188216d28 100644 --- a/src/busctl/busctl.c +++ b/src/busctl/busctl.c @@ -713,8 +713,7 @@ typedef struct Member { uint64_t flags; } Member; -static void member_hash_func(const void *p, struct siphash *state) { - const Member *m = p; +static void member_hash_func(const Member *m, struct siphash *state) { uint64_t arity = 1; assert(m); @@ -921,12 +920,9 @@ static int on_property(const char *interface, const char *name, const char *sign return 0; } -static int introspect(int argc, char **argv, void *userdata) { - static const struct hash_ops member_hash_ops = { - .hash = member_hash_func, - .compare = (__compar_fn_t) member_compare_func, - }; +DEFINE_PRIVATE_HASH_OPS(member_hash_ops, Member, member_hash_func, member_compare_func); +static int introspect(int argc, char **argv, void *userdata) { static const XMLIntrospectOps ops = { .on_interface = on_interface, .on_method = on_method, diff --git a/src/core/socket.c b/src/core/socket.c index f725c9eb00..5540903a69 100644 --- a/src/core/socket.c +++ b/src/core/socket.c @@ -468,9 +468,7 @@ static int socket_verify(Socket *s) { return 0; } -static void peer_address_hash_func(const void *p, struct siphash *state) { - const SocketPeer *s = p; - +static void peer_address_hash_func(const SocketPeer *s, struct siphash *state) { assert(s); if (s->peer.sa.sa_family == AF_INET) @@ -483,8 +481,7 @@ static void peer_address_hash_func(const void *p, struct siphash *state) { assert_not_reached("Unknown address family."); } -static int peer_address_compare_func(const void *a, const void *b) { - const SocketPeer *x = a, *y = b; +static int peer_address_compare_func(const SocketPeer *x, const SocketPeer *y) { int r; r = CMP(x->peer.sa.sa_family, y->peer.sa.sa_family); @@ -502,10 +499,7 @@ static int peer_address_compare_func(const void *a, const void *b) { assert_not_reached("Black sheep in the family!"); } -const struct hash_ops peer_address_hash_ops = { - .hash = peer_address_hash_func, - .compare = peer_address_compare_func -}; +DEFINE_PRIVATE_HASH_OPS(peer_address_hash_ops, SocketPeer, peer_address_hash_func, peer_address_compare_func); static int socket_load(Unit *u) { Socket *s = SOCKET(u); diff --git a/src/journal/catalog.c b/src/journal/catalog.c index 0f18d31add..51efc2b141 100644 --- a/src/journal/catalog.c +++ b/src/journal/catalog.c @@ -49,9 +49,7 @@ typedef struct CatalogItem { le64_t offset; } CatalogItem; -static void catalog_hash_func(const void *p, struct siphash *state) { - const CatalogItem *i = p; - +static void catalog_hash_func(const CatalogItem *i, struct siphash *state) { siphash24_compress(&i->id, sizeof(i->id), state); siphash24_compress(i->language, strlen(i->language), state); } @@ -69,10 +67,7 @@ static int catalog_compare_func(const CatalogItem *a, const CatalogItem *b) { return strcmp(a->language, b->language); } -const struct hash_ops catalog_hash_ops = { - .hash = catalog_hash_func, - .compare = (comparison_fn_t) catalog_compare_func, -}; +DEFINE_HASH_OPS(catalog_hash_ops, CatalogItem, catalog_hash_func, catalog_compare_func); static bool next_header(const char **s) { const char *e; diff --git a/src/libsystemd-network/dhcp-server-internal.h b/src/libsystemd-network/dhcp-server-internal.h index 6355128ddb..8a7c5bc28f 100644 --- a/src/libsystemd-network/dhcp-server-internal.h +++ b/src/libsystemd-network/dhcp-server-internal.h @@ -78,5 +78,5 @@ int dhcp_server_send_packet(sd_dhcp_server *server, DHCPRequest *req, DHCPPacket *packet, int type, size_t optoffset); -void client_id_hash_func(const void *p, struct siphash *state); -int client_id_compare_func(const void *_a, const void *_b); +void client_id_hash_func(const DHCPClientId *p, struct siphash *state); +int client_id_compare_func(const DHCPClientId *a, const DHCPClientId *b); diff --git a/src/libsystemd-network/lldp-neighbor.c b/src/libsystemd-network/lldp-neighbor.c index f3c4e0ac88..023c8e7be6 100644 --- a/src/libsystemd-network/lldp-neighbor.c +++ b/src/libsystemd-network/lldp-neighbor.c @@ -9,9 +9,7 @@ #include "lldp-neighbor.h" #include "unaligned.h" -static void lldp_neighbor_id_hash_func(const void *p, struct siphash *state) { - const LLDPNeighborID *id = p; - +static void lldp_neighbor_id_hash_func(const LLDPNeighborID *id, struct siphash *state) { siphash24_compress(id->chassis_id, id->chassis_id_size, state); siphash24_compress(&id->chassis_id_size, sizeof(id->chassis_id_size), state); siphash24_compress(id->port_id, id->port_id_size, state); @@ -36,10 +34,7 @@ int lldp_neighbor_id_compare_func(const LLDPNeighborID *x, const LLDPNeighborID return CMP(x->port_id_size, y->port_id_size); } -const struct hash_ops lldp_neighbor_id_hash_ops = { - .hash = lldp_neighbor_id_hash_func, - .compare = (__compar_fn_t) lldp_neighbor_id_compare_func, -}; +DEFINE_HASH_OPS(lldp_neighbor_id_hash_ops, LLDPNeighborID, lldp_neighbor_id_hash_func, lldp_neighbor_id_compare_func); int lldp_neighbor_prioq_compare_func(const void *a, const void *b) { const sd_lldp_neighbor *x = a, *y = b; diff --git a/src/libsystemd-network/sd-dhcp-server.c b/src/libsystemd-network/sd-dhcp-server.c index 2a68a02985..072ecf96e6 100644 --- a/src/libsystemd-network/sd-dhcp-server.c +++ b/src/libsystemd-network/sd-dhcp-server.c @@ -102,9 +102,7 @@ int sd_dhcp_server_is_running(sd_dhcp_server *server) { return !!server->receive_message; } -void client_id_hash_func(const void *p, struct siphash *state) { - const DHCPClientId *id = p; - +void client_id_hash_func(const DHCPClientId *id, struct siphash *state) { assert(id); assert(id->length); assert(id->data); @@ -113,13 +111,9 @@ void client_id_hash_func(const void *p, struct siphash *state) { siphash24_compress(id->data, id->length, state); } -int client_id_compare_func(const void *_a, const void *_b) { - const DHCPClientId *a, *b; +int client_id_compare_func(const DHCPClientId *a, const DHCPClientId *b) { int r; - a = _a; - b = _b; - assert(!a->length || a->data); assert(!b->length || b->data); @@ -130,10 +124,7 @@ int client_id_compare_func(const void *_a, const void *_b) { return memcmp(a->data, b->data, a->length); } -static const struct hash_ops client_id_hash_ops = { - .hash = client_id_hash_func, - .compare = client_id_compare_func -}; +DEFINE_PRIVATE_HASH_OPS(client_id_hash_ops, DHCPClientId, client_id_hash_func, client_id_compare_func); static sd_dhcp_server *dhcp_server_free(sd_dhcp_server *server) { DHCPLease *lease; diff --git a/src/libsystemd/sd-bus/bus-objects.c b/src/libsystemd/sd-bus/bus-objects.c index 03c8e56ef8..7e90a56e67 100644 --- a/src/libsystemd/sd-bus/bus-objects.c +++ b/src/libsystemd/sd-bus/bus-objects.c @@ -1582,9 +1582,7 @@ _public_ int sd_bus_add_fallback( return bus_add_object(bus, slot, true, prefix, callback, userdata); } -static void vtable_member_hash_func(const void *a, struct siphash *state) { - const struct vtable_member *m = a; - +static void vtable_member_hash_func(const struct vtable_member *m, struct siphash *state) { assert(m); string_hash_func(m->path, state); @@ -1592,8 +1590,7 @@ static void vtable_member_hash_func(const void *a, struct siphash *state) { string_hash_func(m->member, state); } -static int vtable_member_compare_func(const void *a, const void *b) { - const struct vtable_member *x = a, *y = b; +static int vtable_member_compare_func(const struct vtable_member *x, const struct vtable_member *y) { int r; assert(x); @@ -1610,10 +1607,7 @@ static int vtable_member_compare_func(const void *a, const void *b) { return strcmp(x->member, y->member); } -static const struct hash_ops vtable_member_hash_ops = { - .hash = vtable_member_hash_func, - .compare = vtable_member_compare_func -}; +DEFINE_PRIVATE_HASH_OPS(vtable_member_hash_ops, struct vtable_member, vtable_member_hash_func, vtable_member_compare_func); static int add_object_vtable_internal( sd_bus *bus, diff --git a/src/libsystemd/sd-event/sd-event.c b/src/libsystemd/sd-event/sd-event.c index f0c58a589a..0030ea5dbe 100644 --- a/src/libsystemd/sd-event/sd-event.c +++ b/src/libsystemd/sd-event/sd-event.c @@ -1371,8 +1371,7 @@ static int event_make_inotify_data( return 1; } -static int inode_data_compare(const void *a, const void *b) { - const struct inode_data *x = a, *y = b; +static int inode_data_compare(const struct inode_data *x, const struct inode_data *y) { int r; assert(x); @@ -1385,19 +1384,14 @@ static int inode_data_compare(const void *a, const void *b) { return CMP(x->ino, y->ino); } -static void inode_data_hash_func(const void *p, struct siphash *state) { - const struct inode_data *d = p; - - assert(p); +static void inode_data_hash_func(const struct inode_data *d, struct siphash *state) { + assert(d); siphash24_compress(&d->dev, sizeof(d->dev), state); siphash24_compress(&d->ino, sizeof(d->ino), state); } -const struct hash_ops inode_data_hash_ops = { - .hash = inode_data_hash_func, - .compare = inode_data_compare -}; +DEFINE_PRIVATE_HASH_OPS(inode_data_hash_ops, struct inode_data, inode_data_hash_func, inode_data_compare); static void event_free_inode_data( sd_event *e, diff --git a/src/libsystemd/sd-id128/id128-util.c b/src/libsystemd/sd-id128/id128-util.c index 95c9e1138c..9ffd594ee6 100644 --- a/src/libsystemd/sd-id128/id128-util.c +++ b/src/libsystemd/sd-id128/id128-util.c @@ -182,15 +182,12 @@ int id128_write(const char *p, Id128Format f, sd_id128_t id, bool do_sync) { return id128_write_fd(fd, f, id, do_sync); } -void id128_hash_func(const void *p, struct siphash *state) { - siphash24_compress(p, 16, state); +void id128_hash_func(const sd_id128_t *p, struct siphash *state) { + siphash24_compress(p, sizeof(sd_id128_t), state); } -int id128_compare_func(const void *a, const void *b) { +int id128_compare_func(const sd_id128_t *a, const sd_id128_t *b) { return memcmp(a, b, 16); } -const struct hash_ops id128_hash_ops = { - .hash = id128_hash_func, - .compare = id128_compare_func, -}; +DEFINE_HASH_OPS(id128_hash_ops, sd_id128_t, id128_hash_func, id128_compare_func); diff --git a/src/libsystemd/sd-id128/id128-util.h b/src/libsystemd/sd-id128/id128-util.h index 44f159c0b6..65f14ab252 100644 --- a/src/libsystemd/sd-id128/id128-util.h +++ b/src/libsystemd/sd-id128/id128-util.h @@ -28,6 +28,6 @@ int id128_read(const char *p, Id128Format f, sd_id128_t *ret); int id128_write_fd(int fd, Id128Format f, sd_id128_t id, bool do_sync); int id128_write(const char *p, Id128Format f, sd_id128_t id, bool do_sync); -void id128_hash_func(const void *p, struct siphash *state); -int id128_compare_func(const void *a, const void *b) _pure_; +void id128_hash_func(const sd_id128_t *p, struct siphash *state); +int id128_compare_func(const sd_id128_t *a, const sd_id128_t *b) _pure_; extern const struct hash_ops id128_hash_ops; diff --git a/src/network/networkd-address.c b/src/network/networkd-address.c index 09f041602c..d8e36745c3 100644 --- a/src/network/networkd-address.c +++ b/src/network/networkd-address.c @@ -114,9 +114,7 @@ void address_free(Address *address) { free(address); } -static void address_hash_func(const void *b, struct siphash *state) { - const Address *a = b; - +static void address_hash_func(const Address *a, struct siphash *state) { assert(a); siphash24_compress(&a->family, sizeof(a->family), state); @@ -149,8 +147,7 @@ static void address_hash_func(const void *b, struct siphash *state) { } } -static int address_compare_func(const void *c1, const void *c2) { - const Address *a1 = c1, *a2 = c2; +static int address_compare_func(const Address *a1, const Address *a2) { int r; r = CMP(a1->family, a2->family); @@ -194,10 +191,7 @@ static int address_compare_func(const void *c1, const void *c2) { } } -static const struct hash_ops address_hash_ops = { - .hash = address_hash_func, - .compare = address_compare_func -}; +DEFINE_PRIVATE_HASH_OPS(address_hash_ops, Address, address_hash_func, address_compare_func); bool address_equal(Address *a1, Address *a2) { if (a1 == a2) diff --git a/src/network/networkd-manager.c b/src/network/networkd-manager.c index 3b1ae0b41f..197c37fd0b 100644 --- a/src/network/networkd-manager.c +++ b/src/network/networkd-manager.c @@ -1250,24 +1250,17 @@ static int dhcp6_route_add_handler(sd_netlink *nl, sd_netlink_message *m, void * return 0; } -static void dhcp6_prefixes_hash_func(const void *p, struct siphash *state) { - const struct in6_addr *addr = p; - - assert(p); +static void dhcp6_prefixes_hash_func(const struct in6_addr *addr, struct siphash *state) { + assert(addr); siphash24_compress(addr, sizeof(*addr), state); } -static int dhcp6_prefixes_compare_func(const void *_a, const void *_b) { - const struct in6_addr *a = _a, *b = _b; - +static int dhcp6_prefixes_compare_func(const struct in6_addr *a, const struct in6_addr *b) { return memcmp(a, b, sizeof(*a)); } -static const struct hash_ops dhcp6_prefixes_hash_ops = { - .hash = dhcp6_prefixes_hash_func, - .compare = dhcp6_prefixes_compare_func, -}; +DEFINE_PRIVATE_HASH_OPS(dhcp6_prefixes_hash_ops, struct in6_addr, dhcp6_prefixes_hash_func, dhcp6_prefixes_compare_func); int manager_dhcp6_prefix_add(Manager *m, struct in6_addr *addr, Link *link) { _cleanup_free_ char *buf = NULL; diff --git a/src/network/networkd-ndisc.c b/src/network/networkd-ndisc.c index 8596db11c7..f410e30b65 100644 --- a/src/network/networkd-ndisc.c +++ b/src/network/networkd-ndisc.c @@ -322,22 +322,15 @@ static int ndisc_router_process_route(Link *link, sd_ndisc_router *rt) { return 0; } -static void ndisc_rdnss_hash_func(const void *p, struct siphash *state) { - const NDiscRDNSS *x = p; - +static void ndisc_rdnss_hash_func(const NDiscRDNSS *x, struct siphash *state) { siphash24_compress(&x->address, sizeof(x->address), state); } -static int ndisc_rdnss_compare_func(const void *_a, const void *_b) { - const NDiscRDNSS *a = _a, *b = _b; - +static int ndisc_rdnss_compare_func(const NDiscRDNSS *a, const NDiscRDNSS *b) { return memcmp(&a->address, &b->address, sizeof(a->address)); } -static const struct hash_ops ndisc_rdnss_hash_ops = { - .hash = ndisc_rdnss_hash_func, - .compare = ndisc_rdnss_compare_func -}; +DEFINE_PRIVATE_HASH_OPS(ndisc_rdnss_hash_ops, NDiscRDNSS, ndisc_rdnss_hash_func, ndisc_rdnss_compare_func); static int ndisc_router_process_rdnss(Link *link, sd_ndisc_router *rt) { uint32_t lifetime; @@ -411,22 +404,15 @@ static int ndisc_router_process_rdnss(Link *link, sd_ndisc_router *rt) { return 0; } -static void ndisc_dnssl_hash_func(const void *p, struct siphash *state) { - const NDiscDNSSL *x = p; - +static void ndisc_dnssl_hash_func(const NDiscDNSSL *x, struct siphash *state) { siphash24_compress(NDISC_DNSSL_DOMAIN(x), strlen(NDISC_DNSSL_DOMAIN(x)), state); } -static int ndisc_dnssl_compare_func(const void *_a, const void *_b) { - const NDiscDNSSL *a = _a, *b = _b; - +static int ndisc_dnssl_compare_func(const NDiscDNSSL *a, const NDiscDNSSL *b) { return strcmp(NDISC_DNSSL_DOMAIN(a), NDISC_DNSSL_DOMAIN(b)); } -static const struct hash_ops ndisc_dnssl_hash_ops = { - .hash = ndisc_dnssl_hash_func, - .compare = ndisc_dnssl_compare_func -}; +DEFINE_PRIVATE_HASH_OPS(ndisc_dnssl_hash_ops, NDiscDNSSL, ndisc_dnssl_hash_func, ndisc_dnssl_compare_func); static void ndisc_router_process_dnssl(Link *link, sd_ndisc_router *rt) { _cleanup_strv_free_ char **l = NULL; diff --git a/src/network/networkd-network.c b/src/network/networkd-network.c index 989c92322b..2be63f502e 100644 --- a/src/network/networkd-network.c +++ b/src/network/networkd-network.c @@ -21,15 +21,12 @@ #include "strv.h" #include "util.h" -static void network_config_hash_func(const void *p, struct siphash *state) { - const NetworkConfigSection *c = p; - +static void network_config_hash_func(const NetworkConfigSection *c, struct siphash *state) { siphash24_compress(c->filename, strlen(c->filename), state); siphash24_compress(&c->line, sizeof(c->line), state); } -static int network_config_compare_func(const void *a, const void *b) { - const NetworkConfigSection *x = a, *y = b; +static int network_config_compare_func(const NetworkConfigSection *x, const NetworkConfigSection *y) { int r; r = strcmp(x->filename, y->filename); @@ -39,10 +36,7 @@ static int network_config_compare_func(const void *a, const void *b) { return CMP(x->line, y->line); } -const struct hash_ops network_config_hash_ops = { - .hash = network_config_hash_func, - .compare = network_config_compare_func, -}; +DEFINE_HASH_OPS(network_config_hash_ops, NetworkConfigSection, network_config_hash_func, network_config_compare_func); int network_config_section_new(const char *filename, unsigned line, NetworkConfigSection **s) { NetworkConfigSection *cs; diff --git a/src/network/networkd-route.c b/src/network/networkd-route.c index 2f70844d47..e855f67a26 100644 --- a/src/network/networkd-route.c +++ b/src/network/networkd-route.c @@ -142,9 +142,7 @@ void route_free(Route *route) { free(route); } -static void route_hash_func(const void *b, struct siphash *state) { - const Route *route = b; - +static void route_hash_func(const Route *route, struct siphash *state) { assert(route); siphash24_compress(&route->family, sizeof(route->family), state); @@ -167,8 +165,7 @@ static void route_hash_func(const void *b, struct siphash *state) { } } -static int route_compare_func(const void *_a, const void *_b) { - const Route *a = _a, *b = _b; +static int route_compare_func(const Route *a, const Route *b) { int r; r = CMP(a->family, b->family); @@ -201,10 +198,7 @@ static int route_compare_func(const void *_a, const void *_b) { } } -static const struct hash_ops route_hash_ops = { - .hash = route_hash_func, - .compare = route_compare_func -}; +DEFINE_PRIVATE_HASH_OPS(route_hash_ops, Route, route_hash_func, route_compare_func); bool route_equal(Route *r1, Route *r2) { if (r1 == r2) diff --git a/src/network/networkd-routing-policy-rule.c b/src/network/networkd-routing-policy-rule.c index e84612d41d..b809d825a3 100644 --- a/src/network/networkd-routing-policy-rule.c +++ b/src/network/networkd-routing-policy-rule.c @@ -56,9 +56,7 @@ void routing_policy_rule_free(RoutingPolicyRule *rule) { free(rule); } -static void routing_policy_rule_hash_func(const void *b, struct siphash *state) { - const RoutingPolicyRule *rule = b; - +static void routing_policy_rule_hash_func(const RoutingPolicyRule *rule, struct siphash *state) { assert(rule); siphash24_compress(&rule->family, sizeof(rule->family), state); @@ -94,8 +92,7 @@ static void routing_policy_rule_hash_func(const void *b, struct siphash *state) } } -static int routing_policy_rule_compare_func(const void *_a, const void *_b) { - const RoutingPolicyRule *a = _a, *b = _b; +static int routing_policy_rule_compare_func(const RoutingPolicyRule *a, const RoutingPolicyRule *b) { int r; r = CMP(a->family, b->family); @@ -157,10 +154,7 @@ static int routing_policy_rule_compare_func(const void *_a, const void *_b) { } } -const struct hash_ops routing_policy_rule_hash_ops = { - .hash = routing_policy_rule_hash_func, - .compare = routing_policy_rule_compare_func -}; +DEFINE_PRIVATE_HASH_OPS(routing_policy_rule_hash_ops, RoutingPolicyRule, routing_policy_rule_hash_func, routing_policy_rule_compare_func); int routing_policy_rule_get(Manager *m, int family, diff --git a/src/resolve/resolved-dns-packet.c b/src/resolve/resolved-dns-packet.c index 6eb12f8ae5..0ff444acd0 100644 --- a/src/resolve/resolved-dns-packet.c +++ b/src/resolve/resolved-dns-packet.c @@ -2330,17 +2330,14 @@ int dns_packet_is_reply_for(DnsPacket *p, const DnsResourceKey *key) { return dns_resource_key_equal(p->question->keys[0], key); } -static void dns_packet_hash_func(const void *p, struct siphash *state) { - const DnsPacket *s = p; - +static void dns_packet_hash_func(const DnsPacket *s, struct siphash *state) { assert(s); siphash24_compress(&s->size, sizeof(s->size), state); siphash24_compress(DNS_PACKET_DATA((DnsPacket*) s), s->size, state); } -static int dns_packet_compare_func(const void *a, const void *b) { - const DnsPacket *x = a, *y = b; +static int dns_packet_compare_func(const DnsPacket *x, const DnsPacket *y) { int r; r = CMP(x->size, y->size); @@ -2350,10 +2347,7 @@ static int dns_packet_compare_func(const void *a, const void *b) { return memcmp(DNS_PACKET_DATA((DnsPacket*) x), DNS_PACKET_DATA((DnsPacket*) y), x->size); } -const struct hash_ops dns_packet_hash_ops = { - .hash = dns_packet_hash_func, - .compare = dns_packet_compare_func -}; +DEFINE_HASH_OPS(dns_packet_hash_ops, DnsPacket, dns_packet_hash_func, dns_packet_compare_func); static const char* const dns_rcode_table[_DNS_RCODE_MAX_DEFINED] = { [DNS_RCODE_SUCCESS] = "SUCCESS", diff --git a/src/resolve/resolved-dns-rr.c b/src/resolve/resolved-dns-rr.c index 67565d267e..b67c734e04 100644 --- a/src/resolve/resolved-dns-rr.c +++ b/src/resolve/resolved-dns-rr.c @@ -282,9 +282,7 @@ int dns_resource_key_match_soa(const DnsResourceKey *key, const DnsResourceKey * return dns_name_endswith(dns_resource_key_name(key), dns_resource_key_name(soa)); } -static void dns_resource_key_hash_func(const void *i, struct siphash *state) { - const DnsResourceKey *k = i; - +static void dns_resource_key_hash_func(const DnsResourceKey *k, struct siphash *state) { assert(k); dns_name_hash_func(dns_resource_key_name(k), state); @@ -292,8 +290,7 @@ static void dns_resource_key_hash_func(const void *i, struct siphash *state) { siphash24_compress(&k->type, sizeof(k->type), state); } -static int dns_resource_key_compare_func(const void *a, const void *b) { - const DnsResourceKey *x = a, *y = b; +static int dns_resource_key_compare_func(const DnsResourceKey *x, const DnsResourceKey *y) { int ret; ret = dns_name_compare_func(dns_resource_key_name(x), dns_resource_key_name(y)); @@ -311,10 +308,7 @@ static int dns_resource_key_compare_func(const void *a, const void *b) { return 0; } -const struct hash_ops dns_resource_key_hash_ops = { - .hash = dns_resource_key_hash_func, - .compare = dns_resource_key_compare_func -}; +DEFINE_HASH_OPS(dns_resource_key_hash_ops, DnsResourceKey, dns_resource_key_hash_func, dns_resource_key_compare_func); char* dns_resource_key_to_string(const DnsResourceKey *key, char *buf, size_t buf_size) { const char *c, *t; @@ -1343,9 +1337,7 @@ int dns_resource_record_is_synthetic(DnsResourceRecord *rr) { return !r; } -void dns_resource_record_hash_func(const void *i, struct siphash *state) { - const DnsResourceRecord *rr = i; - +void dns_resource_record_hash_func(const DnsResourceRecord *rr, struct siphash *state) { assert(rr); dns_resource_key_hash_func(rr->key, state); @@ -1486,13 +1478,12 @@ void dns_resource_record_hash_func(const void *i, struct siphash *state) { } } -static int dns_resource_record_compare_func(const void *a, const void *b) { - const DnsResourceRecord *x = a, *y = b; - int ret; +static int dns_resource_record_compare_func(const DnsResourceRecord *x, const DnsResourceRecord *y) { + int r; - ret = dns_resource_key_compare_func(x->key, y->key); - if (ret != 0) - return ret; + r = dns_resource_key_compare_func(x->key, y->key); + if (r != 0) + return r; if (dns_resource_record_equal(x, y)) return 0; @@ -1502,10 +1493,7 @@ static int dns_resource_record_compare_func(const void *a, const void *b) { return CMP(x, y); } -const struct hash_ops dns_resource_record_hash_ops = { - .hash = dns_resource_record_hash_func, - .compare = dns_resource_record_compare_func, -}; +DEFINE_HASH_OPS(dns_resource_record_hash_ops, DnsResourceRecord, dns_resource_record_hash_func, dns_resource_record_compare_func); DnsResourceRecord *dns_resource_record_copy(DnsResourceRecord *rr) { _cleanup_(dns_resource_record_unrefp) DnsResourceRecord *copy = NULL; diff --git a/src/resolve/resolved-dns-rr.h b/src/resolve/resolved-dns-rr.h index d5e74e4937..e02fa2d53b 100644 --- a/src/resolve/resolved-dns-rr.h +++ b/src/resolve/resolved-dns-rr.h @@ -326,7 +326,7 @@ bool dns_txt_item_equal(DnsTxtItem *a, DnsTxtItem *b); DnsTxtItem *dns_txt_item_copy(DnsTxtItem *i); int dns_txt_item_new_empty(DnsTxtItem **ret); -void dns_resource_record_hash_func(const void *i, struct siphash *state); +void dns_resource_record_hash_func(const DnsResourceRecord *i, struct siphash *state); extern const struct hash_ops dns_resource_key_hash_ops; extern const struct hash_ops dns_resource_record_hash_ops; diff --git a/src/resolve/resolved-dns-server.c b/src/resolve/resolved-dns-server.c index be0ae7a731..e05ada29a8 100644 --- a/src/resolve/resolved-dns-server.c +++ b/src/resolve/resolved-dns-server.c @@ -595,9 +595,7 @@ bool dns_server_limited_domains(DnsServer *server) { return domain_restricted; } -static void dns_server_hash_func(const void *p, struct siphash *state) { - const DnsServer *s = p; - +static void dns_server_hash_func(const DnsServer *s, struct siphash *state) { assert(s); siphash24_compress(&s->family, sizeof(s->family), state); @@ -605,8 +603,7 @@ static void dns_server_hash_func(const void *p, struct siphash *state) { siphash24_compress(&s->ifindex, sizeof(s->ifindex), state); } -static int dns_server_compare_func(const void *a, const void *b) { - const DnsServer *x = a, *y = b; +static int dns_server_compare_func(const DnsServer *x, const DnsServer *y) { int r; r = CMP(x->family, y->family); @@ -624,10 +621,7 @@ static int dns_server_compare_func(const void *a, const void *b) { return 0; } -const struct hash_ops dns_server_hash_ops = { - .hash = dns_server_hash_func, - .compare = dns_server_compare_func -}; +DEFINE_HASH_OPS(dns_server_hash_ops, DnsServer, dns_server_hash_func, dns_server_compare_func); void dns_server_unlink_all(DnsServer *first) { DnsServer *next; diff --git a/src/shared/dns-domain.c b/src/shared/dns-domain.c index f24edf5791..f8b2e87ed0 100644 --- a/src/shared/dns-domain.c +++ b/src/shared/dns-domain.c @@ -460,8 +460,7 @@ finish: return 0; } -void dns_name_hash_func(const void *s, struct siphash *state) { - const char *p = s; +void dns_name_hash_func(const char *p, struct siphash *state) { int r; assert(p); @@ -484,15 +483,15 @@ void dns_name_hash_func(const void *s, struct siphash *state) { string_hash_func("", state); } -int dns_name_compare_func(const void *a, const void *b) { +int dns_name_compare_func(const char *a, const char *b) { const char *x, *y; int r, q; assert(a); assert(b); - x = (const char *) a + strlen(a); - y = (const char *) b + strlen(b); + x = a + strlen(a); + y = b + strlen(b); for (;;) { char la[DNS_LABEL_MAX], lb[DNS_LABEL_MAX]; @@ -511,10 +510,7 @@ int dns_name_compare_func(const void *a, const void *b) { } } -const struct hash_ops dns_name_hash_ops = { - .hash = dns_name_hash_func, - .compare = dns_name_compare_func -}; +DEFINE_HASH_OPS(dns_name_hash_ops, char, dns_name_hash_func, dns_name_compare_func); int dns_name_equal(const char *x, const char *y) { int r, q; diff --git a/src/shared/dns-domain.h b/src/shared/dns-domain.h index 1e9b57059b..42492ad7c0 100644 --- a/src/shared/dns-domain.h +++ b/src/shared/dns-domain.h @@ -57,8 +57,8 @@ static inline int dns_name_is_valid(const char *s) { return 1; } -void dns_name_hash_func(const void *s, struct siphash *state); -int dns_name_compare_func(const void *a, const void *b); +void dns_name_hash_func(const char *s, struct siphash *state); +int dns_name_compare_func(const char *a, const char *b); extern const struct hash_ops dns_name_hash_ops; int dns_name_between(const char *a, const char *b, const char *c); diff --git a/src/test/test-prioq.c b/src/test/test-prioq.c index a93d37d460..bc5fdd15b2 100644 --- a/src/test/test-prioq.c +++ b/src/test/test-prioq.c @@ -48,22 +48,15 @@ struct test { unsigned idx; }; -static int test_compare(const void *a, const void *b) { - const struct test *x = a, *y = b; - +static int test_compare(const struct test *x, const struct test *y) { return CMP(x->value, y->value); } -static void test_hash(const void *a, struct siphash *state) { - const struct test *x = a; - +static void test_hash(const struct test *x, struct siphash *state) { siphash24_compress(&x->value, sizeof(x->value), state); } -static const struct hash_ops test_hash_ops = { - .hash = test_hash, - .compare = test_compare -}; +DEFINE_PRIVATE_HASH_OPS(test_hash_ops, struct test, test_hash, test_compare); static void test_struct(void) { _cleanup_(prioq_freep) Prioq *q = NULL; @@ -73,7 +66,7 @@ static void test_struct(void) { srand(0); - assert_se(q = prioq_new(test_compare)); + assert_se(q = prioq_new((compare_func_t) test_compare)); assert_se(s = set_new(&test_hash_ops)); for (i = 0; i < SET_SIZE; i++) { From 59a5cda7b904cd7ef9853bda15b498bbc0577524 Mon Sep 17 00:00:00 2001 From: Yu Watanabe Date: Tue, 27 Nov 2018 22:25:40 +0900 Subject: [PATCH 06/21] hash-func: add destructors for key and value If they are set, then they are called in hashmap_clear() or hashmap_free(). --- src/basic/hash-funcs.h | 53 ++++++++++++++++++++++++-- src/basic/hashmap.c | 84 +++++++++++------------------------------ src/basic/hashmap.h | 50 ++++++++++++++++-------- src/basic/ordered-set.h | 6 +-- src/basic/set.h | 10 ++--- 5 files changed, 113 insertions(+), 90 deletions(-) diff --git a/src/basic/hash-funcs.h b/src/basic/hash-funcs.h index 61c36a2119..3d2ae4b55e 100644 --- a/src/basic/hash-funcs.h +++ b/src/basic/hash-funcs.h @@ -1,6 +1,7 @@ /* SPDX-License-Identifier: LGPL-2.1+ */ #pragma once +#include "alloc-util.h" #include "macro.h" #include "siphash24.h" @@ -10,21 +11,67 @@ typedef int (*compare_func_t)(const void *a, const void *b); struct hash_ops { hash_func_t hash; compare_func_t compare; + free_func_t free_key; + free_func_t free_value; }; -#define _DEFINE_HASH_OPS(uq, name, type, hash_func, compare_func, scope) \ +#define _DEFINE_HASH_OPS(uq, name, type, hash_func, compare_func, free_key_func, free_value_func, scope) \ _unused_ static void (* UNIQ_T(static_hash_wrapper, uq))(const type *, struct siphash *) = hash_func; \ _unused_ static int (* UNIQ_T(static_compare_wrapper, uq))(const type *, const type *) = compare_func; \ scope const struct hash_ops name = { \ .hash = (hash_func_t) hash_func, \ .compare = (compare_func_t) compare_func, \ + .free_key = free_key_func, \ + .free_value = free_value_func, \ } +#define _DEFINE_FREE_FUNC(uq, type, wrapper_name, func) \ + /* Type-safe free function */ \ + static void UNIQ_T(wrapper_name, uq)(void *a) { \ + type *_a = a; \ + func(_a); \ + } + +#define _DEFINE_HASH_OPS_WITH_KEY_DESTRUCTOR(uq, name, type, hash_func, compare_func, free_func, scope) \ + _DEFINE_FREE_FUNC(uq, type, static_free_wrapper, free_func); \ + _DEFINE_HASH_OPS(uq, name, type, hash_func, compare_func, \ + UNIQ_T(static_free_wrapper, uq), NULL, scope) + +#define _DEFINE_HASH_OPS_WITH_VALUE_DESTRUCTOR(uq, name, type, hash_func, compare_func, type_value, free_func, scope) \ + _DEFINE_FREE_FUNC(uq, type_value, static_free_wrapper, free_func); \ + _DEFINE_HASH_OPS(uq, name, type, hash_func, compare_func, \ + NULL, UNIQ_T(static_free_wrapper, uq), scope) + +#define _DEFINE_HASH_OPS_FULL(uq, name, type, hash_func, compare_func, free_key_func, type_value, free_value_func, scope) \ + _DEFINE_FREE_FUNC(uq, type, static_free_key_wrapper, free_key_func); \ + _DEFINE_FREE_FUNC(uq, type_value, static_free_value_wrapper, free_value_func); \ + _DEFINE_HASH_OPS(uq, name, type, hash_func, compare_func, \ + UNIQ_T(static_free_key_wrapper, uq), \ + UNIQ_T(static_free_value_wrapper, uq), scope) + #define DEFINE_HASH_OPS(name, type, hash_func, compare_func) \ - _DEFINE_HASH_OPS(UNIQ, name, type, hash_func, compare_func,) + _DEFINE_HASH_OPS(UNIQ, name, type, hash_func, compare_func, NULL, NULL,) #define DEFINE_PRIVATE_HASH_OPS(name, type, hash_func, compare_func) \ - _DEFINE_HASH_OPS(UNIQ, name, type, hash_func, compare_func, static) + _DEFINE_HASH_OPS(UNIQ, name, type, hash_func, compare_func, NULL, NULL, static) + +#define DEFINE_HASH_OPS_WITH_KEY_DESTRUCTOR(name, type, hash_func, compare_func, free_func) \ + _DEFINE_HASH_OPS_WITH_KEY_DESTRUCTOR(UNIQ, name, type, hash_func, compare_func, free_func,) + +#define DEFINE_PRIVATE_HASH_OPS_WITH_KEY_DESTRUCTOR(name, type, hash_func, compare_func, free_func) \ + _DEFINE_HASH_OPS_WITH_KEY_DESTRUCTOR(UNIQ, name, type, hash_func, compare_func, free_func, static) + +#define DEFINE_HASH_OPS_WITH_VALUE_DESTRUCTOR(name, type, hash_func, compare_func, value_type, free_func) \ + _DEFINE_HASH_OPS_WITH_VALUE_DESTRUCTOR(UNIQ, name, type, hash_func, compare_func, value_type, free_func,) + +#define DEFINE_PRIVATE_HASH_OPS_WITH_VALUE_DESTRUCTOR(name, type, hash_func, compare_func, value_type, free_func) \ + _DEFINE_HASH_OPS_WITH_VALUE_DESTRUCTOR(UNIQ, name, type, hash_func, compare_func, value_type, free_func, static) + +#define DEFINE_HASH_OPS_FULL(name, type, hash_func, compare_func, free_key_func, value_type, free_value_func) \ + _DEFINE_HASH_OPS_FULL(UNIQ, name, type, hash_func, compare_func, free_key_func, value_type, free_value_func,) + +#define DEFINE_PRIVATE_HASH_OPS_FULL(name, type, hash_func, compare_func, free_key_func, value_type, free_value_func) \ + _DEFINE_HASH_OPS_FULL(UNIQ, name, type, hash_func, compare_func, free_key_func, value_type, free_value_func, static) void string_hash_func(const char *p, struct siphash *state); #define string_compare_func strcmp diff --git a/src/basic/hashmap.c b/src/basic/hashmap.c index a59640023f..5a676eeb61 100644 --- a/src/basic/hashmap.c +++ b/src/basic/hashmap.c @@ -864,47 +864,38 @@ static void hashmap_free_no_clear(HashmapBase *h) { free(h); } -HashmapBase *internal_hashmap_free(HashmapBase *h) { - - /* Free the hashmap, but nothing in it */ - +HashmapBase *internal_hashmap_free(HashmapBase *h, free_func_t default_free_key, free_func_t default_free_value) { if (h) { - internal_hashmap_clear(h); + internal_hashmap_clear(h, default_free_key, default_free_value); hashmap_free_no_clear(h); } return NULL; } -HashmapBase *internal_hashmap_free_free(HashmapBase *h) { - - /* Free the hashmap and all data objects in it, but not the - * keys */ - - if (h) { - internal_hashmap_clear_free(h); - hashmap_free_no_clear(h); - } - - return NULL; -} - -Hashmap *hashmap_free_free_free(Hashmap *h) { - - /* Free the hashmap and all data and key objects in it */ - - if (h) { - hashmap_clear_free_free(h); - hashmap_free_no_clear(HASHMAP_BASE(h)); - } - - return NULL; -} - -void internal_hashmap_clear(HashmapBase *h) { +void internal_hashmap_clear(HashmapBase *h, free_func_t default_free_key, free_func_t default_free_value) { + free_func_t free_key, free_value; if (!h) return; + free_key = h->hash_ops->free_key ?: default_free_key; + free_value = h->hash_ops->free_value ?: default_free_value; + + if (free_key || free_value) { + unsigned idx; + + for (idx = skip_free_buckets(h, 0); idx != IDX_NIL; + idx = skip_free_buckets(h, idx + 1)) { + struct hashmap_base_entry *e = bucket_at(h, idx); + + if (free_key) + free_key((void *) e->key); + + if (free_value) + free_value(entry_value(h, e)); + } + } + if (h->has_indirect) { free(h->indirect.storage); h->has_indirect = false; @@ -921,35 +912,6 @@ void internal_hashmap_clear(HashmapBase *h) { base_set_dirty(h); } -void internal_hashmap_clear_free(HashmapBase *h) { - unsigned idx; - - if (!h) - return; - - for (idx = skip_free_buckets(h, 0); idx != IDX_NIL; - idx = skip_free_buckets(h, idx + 1)) - free(entry_value(h, bucket_at(h, idx))); - - internal_hashmap_clear(h); -} - -void hashmap_clear_free_free(Hashmap *h) { - unsigned idx; - - if (!h) - return; - - for (idx = skip_free_buckets(HASHMAP_BASE(h), 0); idx != IDX_NIL; - idx = skip_free_buckets(HASHMAP_BASE(h), idx + 1)) { - struct plain_hashmap_entry *e = plain_bucket_at(h, idx); - free((void*)e->b.key); - free(e->value); - } - - internal_hashmap_clear(HASHMAP_BASE(h)); -} - static int resize_buckets(HashmapBase *h, unsigned entries_add); /* @@ -1740,7 +1702,7 @@ HashmapBase *internal_hashmap_copy(HashmapBase *h) { } if (r < 0) { - internal_hashmap_free(copy); + internal_hashmap_free(copy, false, false); return NULL; } diff --git a/src/basic/hashmap.h b/src/basic/hashmap.h index bb2a5c76ec..a330a1d494 100644 --- a/src/basic/hashmap.h +++ b/src/basic/hashmap.h @@ -22,6 +22,8 @@ #define HASH_KEY_SIZE 16 +typedef void* (*hashmap_destroy_t)(void *p); + /* The base type for all hashmap and set types. Many functions in the * implementation take (HashmapBase*) parameters and are run-time polymorphic, * though the API is not meant to be polymorphic (do not call functions @@ -87,25 +89,33 @@ OrderedHashmap *internal_ordered_hashmap_new(const struct hash_ops *hash_ops HA #define hashmap_new(ops) internal_hashmap_new(ops HASHMAP_DEBUG_SRC_ARGS) #define ordered_hashmap_new(ops) internal_ordered_hashmap_new(ops HASHMAP_DEBUG_SRC_ARGS) -HashmapBase *internal_hashmap_free(HashmapBase *h); +HashmapBase *internal_hashmap_free(HashmapBase *h, free_func_t default_free_key, free_func_t default_free_value); static inline Hashmap *hashmap_free(Hashmap *h) { - return (void*)internal_hashmap_free(HASHMAP_BASE(h)); + return (void*) internal_hashmap_free(HASHMAP_BASE(h), NULL, NULL); } static inline OrderedHashmap *ordered_hashmap_free(OrderedHashmap *h) { - return (void*)internal_hashmap_free(HASHMAP_BASE(h)); + return (void*) internal_hashmap_free(HASHMAP_BASE(h), NULL, NULL); } -HashmapBase *internal_hashmap_free_free(HashmapBase *h); static inline Hashmap *hashmap_free_free(Hashmap *h) { - return (void*)internal_hashmap_free_free(HASHMAP_BASE(h)); + return (void*) internal_hashmap_free(HASHMAP_BASE(h), NULL, free); } static inline OrderedHashmap *ordered_hashmap_free_free(OrderedHashmap *h) { - return (void*)internal_hashmap_free_free(HASHMAP_BASE(h)); + return (void*) internal_hashmap_free(HASHMAP_BASE(h), NULL, free); } -Hashmap *hashmap_free_free_free(Hashmap *h); +static inline Hashmap *hashmap_free_free_key(Hashmap *h) { + return (void*) internal_hashmap_free(HASHMAP_BASE(h), free, NULL); +} +static inline OrderedHashmap *ordered_hashmap_free_free_key(OrderedHashmap *h) { + return (void*) internal_hashmap_free(HASHMAP_BASE(h), free, NULL); +} + +static inline Hashmap *hashmap_free_free_free(Hashmap *h) { + return (void*) internal_hashmap_free(HASHMAP_BASE(h), free, free); +} static inline OrderedHashmap *ordered_hashmap_free_free_free(OrderedHashmap *h) { - return (void*)hashmap_free_free_free(PLAIN_HASHMAP(h)); + return (void*) internal_hashmap_free(HASHMAP_BASE(h), free, free); } IteratedCache *iterated_cache_free(IteratedCache *cache); @@ -258,25 +268,33 @@ static inline bool ordered_hashmap_iterate(OrderedHashmap *h, Iterator *i, void return internal_hashmap_iterate(HASHMAP_BASE(h), i, value, key); } -void internal_hashmap_clear(HashmapBase *h); +void internal_hashmap_clear(HashmapBase *h, free_func_t default_free_key, free_func_t default_free_value); static inline void hashmap_clear(Hashmap *h) { - internal_hashmap_clear(HASHMAP_BASE(h)); + internal_hashmap_clear(HASHMAP_BASE(h), NULL, NULL); } static inline void ordered_hashmap_clear(OrderedHashmap *h) { - internal_hashmap_clear(HASHMAP_BASE(h)); + internal_hashmap_clear(HASHMAP_BASE(h), NULL, NULL); } -void internal_hashmap_clear_free(HashmapBase *h); static inline void hashmap_clear_free(Hashmap *h) { - internal_hashmap_clear_free(HASHMAP_BASE(h)); + internal_hashmap_clear(HASHMAP_BASE(h), NULL, free); } static inline void ordered_hashmap_clear_free(OrderedHashmap *h) { - internal_hashmap_clear_free(HASHMAP_BASE(h)); + internal_hashmap_clear(HASHMAP_BASE(h), NULL, free); } -void hashmap_clear_free_free(Hashmap *h); +static inline void hashmap_clear_free_key(Hashmap *h) { + internal_hashmap_clear(HASHMAP_BASE(h), free, NULL); +} +static inline void ordered_hashmap_clear_free_key(OrderedHashmap *h) { + internal_hashmap_clear(HASHMAP_BASE(h), free, NULL); +} + +static inline void hashmap_clear_free_free(Hashmap *h) { + internal_hashmap_clear(HASHMAP_BASE(h), free, free); +} static inline void ordered_hashmap_clear_free_free(OrderedHashmap *h) { - hashmap_clear_free_free(PLAIN_HASHMAP(h)); + internal_hashmap_clear(HASHMAP_BASE(h), free, free); } /* diff --git a/src/basic/ordered-set.h b/src/basic/ordered-set.h index e7c054d8e4..7cbb71819b 100644 --- a/src/basic/ordered-set.h +++ b/src/basic/ordered-set.h @@ -21,13 +21,11 @@ static inline int ordered_set_ensure_allocated(OrderedSet **s, const struct hash } static inline OrderedSet* ordered_set_free(OrderedSet *s) { - ordered_hashmap_free((OrderedHashmap*) s); - return NULL; + return (OrderedSet*) ordered_hashmap_free((OrderedHashmap*) s); } static inline OrderedSet* ordered_set_free_free(OrderedSet *s) { - ordered_hashmap_free_free((OrderedHashmap*) s); - return NULL; + return (OrderedSet*) ordered_hashmap_free_free((OrderedHashmap*) s); } static inline int ordered_set_put(OrderedSet *s, void *p) { diff --git a/src/basic/set.h b/src/basic/set.h index 0d99d5550d..2a80632b79 100644 --- a/src/basic/set.h +++ b/src/basic/set.h @@ -9,13 +9,11 @@ Set *internal_set_new(const struct hash_ops *hash_ops HASHMAP_DEBUG_PARAMS); #define set_new(ops) internal_set_new(ops HASHMAP_DEBUG_SRC_ARGS) static inline Set *set_free(Set *s) { - internal_hashmap_free(HASHMAP_BASE(s)); - return NULL; + return (Set*) internal_hashmap_free(HASHMAP_BASE(s), NULL, NULL); } static inline Set *set_free_free(Set *s) { - internal_hashmap_free_free(HASHMAP_BASE(s)); - return NULL; + return (Set*) internal_hashmap_free(HASHMAP_BASE(s), free, NULL); } /* no set_free_free_free */ @@ -76,11 +74,11 @@ static inline unsigned set_buckets(Set *s) { bool set_iterate(Set *s, Iterator *i, void **value); static inline void set_clear(Set *s) { - internal_hashmap_clear(HASHMAP_BASE(s)); + internal_hashmap_clear(HASHMAP_BASE(s), NULL, NULL); } static inline void set_clear_free(Set *s) { - internal_hashmap_clear_free(HASHMAP_BASE(s)); + internal_hashmap_clear(HASHMAP_BASE(s), free, NULL); } /* no set_clear_free_free */ From 98233ee5e031cf39f5be73651a1f05c52927116b Mon Sep 17 00:00:00 2001 From: Yu Watanabe Date: Sun, 25 Nov 2018 21:54:44 +0900 Subject: [PATCH 07/21] test: add tests for destructors of hashmap or set --- src/test/test-hashmap-plain.c | 38 +++++++++++++++++++++++++++++++++++ src/test/test-set.c | 19 ++++++++++++++++++ 2 files changed, 57 insertions(+) diff --git a/src/test/test-hashmap-plain.c b/src/test/test-hashmap-plain.c index 82837da4f6..c04ef0b1f7 100644 --- a/src/test/test-hashmap-plain.c +++ b/src/test/test-hashmap-plain.c @@ -863,6 +863,43 @@ static void test_hashmap_clear_free_free(void) { hashmap_clear_free_free(m); assert_se(hashmap_isempty(m)); + + assert_se(hashmap_put(m, strdup("key 1"), strdup("value 1")) == 1); + assert_se(hashmap_put(m, strdup("key 2"), strdup("value 2")) == 1); + assert_se(hashmap_put(m, strdup("key 3"), strdup("value 3")) == 1); + + hashmap_clear_free_free(m); + assert_se(hashmap_isempty(m)); +} + +DEFINE_PRIVATE_HASH_OPS_WITH_KEY_DESTRUCTOR(test_hash_ops_key, char, string_hash_func, string_compare_func, free); +DEFINE_PRIVATE_HASH_OPS_FULL(test_hash_ops_full, char, string_hash_func, string_compare_func, free, char, free); + +static void test_hashmap_clear_free_with_destructor(void) { + _cleanup_hashmap_free_ Hashmap *m = NULL; + + log_info("%s", __func__); + + m = hashmap_new(&test_hash_ops_key); + assert_se(m); + + assert_se(hashmap_put(m, strdup("key 1"), NULL) == 1); + assert_se(hashmap_put(m, strdup("key 2"), NULL) == 1); + assert_se(hashmap_put(m, strdup("key 3"), NULL) == 1); + + hashmap_clear_free(m); + assert_se(hashmap_isempty(m)); + m = hashmap_free(m); + + m = hashmap_new(&test_hash_ops_full); + assert_se(m); + + assert_se(hashmap_put(m, strdup("key 1"), strdup("value 1")) == 1); + assert_se(hashmap_put(m, strdup("key 2"), strdup("value 2")) == 1); + assert_se(hashmap_put(m, strdup("key 3"), strdup("value 3")) == 1); + + hashmap_clear_free(m); + assert_se(hashmap_isempty(m)); } static void test_hashmap_reserve(void) { @@ -915,5 +952,6 @@ void test_hashmap_funcs(void) { test_hashmap_steal_first_key(); test_hashmap_steal_first(); test_hashmap_clear_free_free(); + test_hashmap_clear_free_with_destructor(); test_hashmap_reserve(); } diff --git a/src/test/test-set.c b/src/test/test-set.c index 6307403e4c..340edeb65f 100644 --- a/src/test/test-set.c +++ b/src/test/test-set.c @@ -45,6 +45,24 @@ static void test_set_free_with_destructor(void) { assert_se(items[3].seen == 0); } +DEFINE_PRIVATE_HASH_OPS_WITH_VALUE_DESTRUCTOR(item_hash_ops, void, trivial_hash_func, trivial_compare_func, Item, item_seen); + +static void test_set_free_with_hash_ops(void) { + Set *m; + struct Item items[4] = {}; + unsigned i; + + assert_se(m = set_new(&item_hash_ops)); + for (i = 0; i < ELEMENTSOF(items) - 1; i++) + assert_se(set_put(m, items + i) == 1); + + m = set_free(m); + assert_se(items[0].seen == 1); + assert_se(items[1].seen == 1); + assert_se(items[2].seen == 1); + assert_se(items[3].seen == 0); +} + static void test_set_put(void) { _cleanup_set_free_ Set *m = NULL; @@ -64,6 +82,7 @@ static void test_set_put(void) { int main(int argc, const char *argv[]) { test_set_steal_first(); test_set_free_with_destructor(); + test_set_free_with_hash_ops(); test_set_put(); return 0; From fb8d181e5918f3d2dc38cb9d6f64aac9ce2e6367 Mon Sep 17 00:00:00 2001 From: Yu Watanabe Date: Sun, 25 Nov 2018 23:54:19 +0900 Subject: [PATCH 08/21] cgtop: introduce group_hash_ops and use it --- src/cgtop/cgtop.c | 28 ++++++++++------------------ 1 file changed, 10 insertions(+), 18 deletions(-) diff --git a/src/cgtop/cgtop.c b/src/cgtop/cgtop.c index 5f6e778a38..6a821ff38e 100644 --- a/src/cgtop/cgtop.c +++ b/src/cgtop/cgtop.c @@ -84,24 +84,14 @@ static enum { CPU_TIME, } arg_cpu_type = CPU_PERCENT; -static void group_free(Group *g) { - assert(g); +static Group *group_free(Group *g) { + if (!g) + return NULL; free(g->path); - free(g); + return mfree(g); } -static void group_hashmap_clear(Hashmap *h) { - hashmap_clear_with_destructor(h, group_free); -} - -static void group_hashmap_free(Hashmap *h) { - group_hashmap_clear(h); - hashmap_free(h); -} - -DEFINE_TRIVIAL_CLEANUP_FUNC(Hashmap*, group_hashmap_free); - static const char *maybe_format_bytes(char *buf, size_t l, bool is_valid, uint64_t t) { if (!is_valid) return "-"; @@ -908,8 +898,10 @@ static const char* counting_what(void) { return "userspace processes (excl. kernel)"; } +DEFINE_PRIVATE_HASH_OPS_WITH_VALUE_DESTRUCTOR(group_hash_ops, char, path_hash_func, path_compare_func, Group, group_free); + static int run(int argc, char *argv[]) { - _cleanup_(group_hashmap_freep) Hashmap *a = NULL, *b = NULL; + _cleanup_hashmap_free_ Hashmap *a = NULL, *b = NULL; unsigned iteration = 0; usec_t last_refresh = 0; bool quit = false, immediate_refresh = false; @@ -940,8 +932,8 @@ static int run(int argc, char *argv[]) { return log_error_errno(r, "Failed to get root control group path: %m"); log_debug("Cgroup path: %s", root); - a = hashmap_new(&path_hash_ops); - b = hashmap_new(&path_hash_ops); + a = hashmap_new(&group_hash_ops); + b = hashmap_new(&group_hash_ops); if (!a || !b) return log_oom(); @@ -964,7 +956,7 @@ static int run(int argc, char *argv[]) { if (r < 0) return log_error_errno(r, "Failed to refresh: %m"); - group_hashmap_clear(b); + hashmap_clear(b); c = a; a = b; From 5f5b61220341b3c803fafb9522e3536ce43708cf Mon Sep 17 00:00:00 2001 From: Yu Watanabe Date: Sun, 25 Nov 2018 23:55:30 +0900 Subject: [PATCH 09/21] cgtop: use SWAP_TWO() macro --- src/cgtop/cgtop.c | 6 +----- 1 file changed, 1 insertion(+), 5 deletions(-) diff --git a/src/cgtop/cgtop.c b/src/cgtop/cgtop.c index 6a821ff38e..5abfab03de 100644 --- a/src/cgtop/cgtop.c +++ b/src/cgtop/cgtop.c @@ -943,7 +943,6 @@ static int run(int argc, char *argv[]) { arg_iterations = on_tty() ? 0 : 1; while (!quit) { - Hashmap *c; usec_t t; char key; char h[FORMAT_TIMESPAN_MAX]; @@ -957,10 +956,7 @@ static int run(int argc, char *argv[]) { return log_error_errno(r, "Failed to refresh: %m"); hashmap_clear(b); - - c = a; - a = b; - b = c; + SWAP_TWO(a, b); last_refresh = t; immediate_refresh = false; From 6121fc3059e0cb88de0b8f4756da5d4416d2cad8 Mon Sep 17 00:00:00 2001 From: Yu Watanabe Date: Sun, 25 Nov 2018 23:58:49 +0900 Subject: [PATCH 10/21] dhcp: introduce dhcp_lease_hash_ops and use it --- src/libsystemd-network/sd-dhcp-server.c | 17 +++++++---------- 1 file changed, 7 insertions(+), 10 deletions(-) diff --git a/src/libsystemd-network/sd-dhcp-server.c b/src/libsystemd-network/sd-dhcp-server.c index 072ecf96e6..7cb44d1fdf 100644 --- a/src/libsystemd-network/sd-dhcp-server.c +++ b/src/libsystemd-network/sd-dhcp-server.c @@ -21,12 +21,12 @@ #define DHCP_DEFAULT_LEASE_TIME_USEC USEC_PER_HOUR #define DHCP_MAX_LEASE_TIME_USEC (USEC_PER_HOUR*12) -static void dhcp_lease_free(DHCPLease *lease) { +static DHCPLease *dhcp_lease_free(DHCPLease *lease) { if (!lease) - return; + return NULL; free(lease->client_id.data); - free(lease); + return mfree(lease); } /* configures the server's address and subnet, and optionally the pool's size and offset into the subnet @@ -90,7 +90,7 @@ int sd_dhcp_server_configure_pool(sd_dhcp_server *server, struct in_addr *addres server->bound_leases[server_off - offset] = &server->invalid_lease; /* Drop any leases associated with the old address range */ - hashmap_clear_with_destructor(server->leases_by_client_id, dhcp_lease_free); + hashmap_clear(server->leases_by_client_id); } return 0; @@ -124,11 +124,10 @@ int client_id_compare_func(const DHCPClientId *a, const DHCPClientId *b) { return memcmp(a->data, b->data, a->length); } -DEFINE_PRIVATE_HASH_OPS(client_id_hash_ops, DHCPClientId, client_id_hash_func, client_id_compare_func); +DEFINE_PRIVATE_HASH_OPS_WITH_VALUE_DESTRUCTOR(dhcp_lease_hash_ops, DHCPClientId, client_id_hash_func, client_id_compare_func, + DHCPLease, dhcp_lease_free); static sd_dhcp_server *dhcp_server_free(sd_dhcp_server *server) { - DHCPLease *lease; - assert(server); log_dhcp_server(server, "UNREF"); @@ -141,8 +140,6 @@ static sd_dhcp_server *dhcp_server_free(sd_dhcp_server *server) { free(server->dns); free(server->ntp); - while ((lease = hashmap_steal_first(server->leases_by_client_id))) - dhcp_lease_free(lease); hashmap_free(server->leases_by_client_id); free(server->bound_leases); @@ -168,7 +165,7 @@ int sd_dhcp_server_new(sd_dhcp_server **ret, int ifindex) { server->netmask = htobe32(INADDR_ANY); server->ifindex = ifindex; - server->leases_by_client_id = hashmap_new(&client_id_hash_ops); + server->leases_by_client_id = hashmap_new(&dhcp_lease_hash_ops); if (!server->leases_by_client_id) return -ENOMEM; From f1bcdd05907f37e60196843d991bfc6ffc9a6c77 Mon Sep 17 00:00:00 2001 From: Yu Watanabe Date: Mon, 26 Nov 2018 00:04:46 +0900 Subject: [PATCH 11/21] sd-bus: introduce track_item_hash_ops and use it --- src/libsystemd/sd-bus/bus-track.c | 8 +++++--- 1 file changed, 5 insertions(+), 3 deletions(-) diff --git a/src/libsystemd/sd-bus/bus-track.c b/src/libsystemd/sd-bus/bus-track.c index 4c959cc29c..efbd3ed5f0 100644 --- a/src/libsystemd/sd-bus/bus-track.c +++ b/src/libsystemd/sd-bus/bus-track.c @@ -50,6 +50,8 @@ static struct track_item* track_item_free(struct track_item *i) { } DEFINE_TRIVIAL_CLEANUP_FUNC(struct track_item*, track_item_free); +DEFINE_PRIVATE_HASH_OPS_WITH_VALUE_DESTRUCTOR(track_item_hash_ops, char, string_hash_func, string_compare_func, + struct track_item, track_item_free); static void bus_track_add_to_queue(sd_bus_track *track) { assert(track); @@ -150,7 +152,7 @@ static sd_bus_track *track_free(sd_bus_track *track) { LIST_REMOVE(tracks, track->bus->tracks, track); bus_track_remove_from_queue(track); - track->names = hashmap_free_with_destructor(track->names, track_item_free); + track->names = hashmap_free(track->names); track->bus = sd_bus_unref(track->bus); if (track->destroy_callback) @@ -201,7 +203,7 @@ _public_ int sd_bus_track_add_name(sd_bus_track *track, const char *name) { return 0; } - r = hashmap_ensure_allocated(&track->names, &string_hash_ops); + r = hashmap_ensure_allocated(&track->names, &track_item_hash_ops); if (r < 0) return r; @@ -397,7 +399,7 @@ void bus_track_close(sd_bus_track *track) { return; /* Let's flush out all names */ - hashmap_clear_with_destructor(track->names, track_item_free); + hashmap_clear(track->names); /* Invoke handler */ if (track->handler) From a4a90e65c7921ce250487dd596aea65fcd192766 Mon Sep 17 00:00:00 2001 From: Yu Watanabe Date: Mon, 26 Nov 2018 00:19:08 +0900 Subject: [PATCH 12/21] cryptsetup-generator: define main through macro --- src/cryptsetup/cryptsetup-generator.c | 58 +++++++++++++-------------- 1 file changed, 28 insertions(+), 30 deletions(-) diff --git a/src/cryptsetup/cryptsetup-generator.c b/src/cryptsetup/cryptsetup-generator.c index b0b1065e2f..8a889e980a 100644 --- a/src/cryptsetup/cryptsetup-generator.c +++ b/src/cryptsetup/cryptsetup-generator.c @@ -13,6 +13,7 @@ #include "hashmap.h" #include "id128-util.h" #include "log.h" +#include "main-func.h" #include "mkdir.h" #include "parse-util.h" #include "path-util.h" @@ -40,6 +41,10 @@ static Hashmap *arg_disks = NULL; static char *arg_default_options = NULL; static char *arg_default_keyfile = NULL; +STATIC_DESTRUCTOR_REGISTER(arg_disks, hashmap_freep); +STATIC_DESTRUCTOR_REGISTER(arg_default_options, freep); +STATIC_DESTRUCTOR_REGISTER(arg_default_keyfile, freep); + static int generate_keydev_mount(const char *name, const char *keydev, char **unit, char **mount) { _cleanup_free_ char *u = NULL, *what = NULL, *where = NULL, *name_escaped = NULL; _cleanup_fclose_ FILE *f = NULL; @@ -310,13 +315,16 @@ static int create_disk( return 0; } -static void crypt_device_free(crypto_device *d) { +static crypto_device* crypt_device_free(crypto_device *d) { + if (!d) + return NULL; + free(d->uuid); free(d->keyfile); free(d->keydev); free(d->name); free(d->options); - free(d); + return mfree(d); } static crypto_device *get_crypto_device(const char *uuid) { @@ -569,50 +577,40 @@ static int add_proc_cmdline_devices(void) { return 0; } -int main(int argc, char *argv[]) { +DEFINE_PRIVATE_HASH_OPS_WITH_VALUE_DESTRUCTOR(crypt_device_hash_ops, char, string_hash_func, string_compare_func, + crypto_device, crypt_device_free); + +static int run(int argc, char *argv[]) { int r; - if (argc > 1 && argc != 4) { - log_error("This program takes three or no arguments."); - return EXIT_FAILURE; - } + if (argc > 1 && argc != 4) + return log_error_errno(SYNTHETIC_ERRNO(EINVAL), "This program takes three or no arguments."); if (argc > 1) arg_dest = argv[1]; log_setup_generator(); - arg_disks = hashmap_new(&string_hash_ops); - if (!arg_disks) { - r = log_oom(); - goto finish; - } + arg_disks = hashmap_new(&crypt_device_hash_ops); + if (!arg_disks) + return log_oom(); r = proc_cmdline_parse(parse_proc_cmdline_item, NULL, PROC_CMDLINE_STRIP_RD_PREFIX); - if (r < 0) { - log_warning_errno(r, "Failed to parse kernel command line: %m"); - goto finish; - } + if (r < 0) + return log_warning_errno(r, "Failed to parse kernel command line: %m"); - if (!arg_enabled) { - r = 0; - goto finish; - } + if (!arg_enabled) + return 0; r = add_crypttab_devices(); if (r < 0) - goto finish; + return r; r = add_proc_cmdline_devices(); if (r < 0) - goto finish; + return r; - r = 0; - -finish: - hashmap_free_with_destructor(arg_disks, crypt_device_free); - free(arg_default_options); - free(arg_default_keyfile); - - return r < 0 ? EXIT_FAILURE : EXIT_SUCCESS; + return 0; } + +DEFINE_MAIN_FUNCTION(run); From 71a0be76cd8f2010b747d88c471af19fe162c5f7 Mon Sep 17 00:00:00 2001 From: Yu Watanabe Date: Mon, 26 Nov 2018 00:48:15 +0900 Subject: [PATCH 13/21] tmpfiles: make load_unix_sockets() return negative errno on failure --- src/tmpfiles/tmpfiles.c | 76 +++++++++++++++++------------------------ 1 file changed, 31 insertions(+), 45 deletions(-) diff --git a/src/tmpfiles/tmpfiles.c b/src/tmpfiles/tmpfiles.c index ce17d6972d..b673ce1080 100644 --- a/src/tmpfiles/tmpfiles.c +++ b/src/tmpfiles/tmpfiles.c @@ -372,48 +372,39 @@ static struct Item* find_glob(OrderedHashmap *h, const char *match) { return NULL; } -static void load_unix_sockets(void) { +static int load_unix_sockets(void) { + _cleanup_set_free_free_ Set *sockets = NULL; _cleanup_fclose_ FILE *f = NULL; int r; if (unix_sockets) - return; + return 0; /* We maintain a cache of the sockets we found in /proc/net/unix to speed things up a little. */ - unix_sockets = set_new(&path_hash_ops); - if (!unix_sockets) { - log_oom(); - return; - } + sockets = set_new(&path_hash_ops); + if (!sockets) + return log_oom(); f = fopen("/proc/net/unix", "re"); - if (!f) { - log_full_errno(errno == ENOENT ? LOG_DEBUG : LOG_WARNING, errno, - "Failed to open /proc/net/unix, ignoring: %m"); - goto fail; - } + if (!f) + return log_full_errno(errno == ENOENT ? LOG_DEBUG : LOG_WARNING, errno, + "Failed to open /proc/net/unix, ignoring: %m"); /* Skip header */ r = read_line(f, LONG_LINE_MAX, NULL); - if (r < 0) { - log_warning_errno(r, "Failed to skip /proc/net/unix header line: %m"); - goto fail; - } - if (r == 0) { - log_warning("Premature end of file reading /proc/net/unix."); - goto fail; - } + if (r < 0) + return log_warning_errno(r, "Failed to skip /proc/net/unix header line: %m"); + if (r == 0) + return log_warning_errno(SYNTHETIC_ERRNO(EIO), "Premature end of file reading /proc/net/unix."); for (;;) { - _cleanup_free_ char *line = NULL; - char *p, *s; + _cleanup_free_ char *line = NULL, *s = NULL; + char *p; r = read_line(f, LONG_LINE_MAX, &line); - if (r < 0) { - log_warning_errno(r, "Failed to read /proc/net/unix line, ignoring: %m"); - goto fail; - } + if (r < 0) + return log_warning_errno(r, "Failed to read /proc/net/unix line, ignoring: %m"); if (r == 0) /* EOF */ break; @@ -433,36 +424,31 @@ static void load_unix_sockets(void) { continue; s = strdup(p); - if (!s) { - log_oom(); - goto fail; - } + if (!s) + return log_oom(); path_simplify(s, false); - r = set_consume(unix_sockets, s); - if (r < 0 && r != -EEXIST) { - log_warning_errno(r, "Failed to add AF_UNIX socket to set, ignoring: %m"); - goto fail; - } + r = set_consume(sockets, s); + if (r == -EEXIST) + continue; + if (r < 0) + return log_warning_errno(r, "Failed to add AF_UNIX socket to set, ignoring: %m"); + + TAKE_PTR(s); } - return; - -fail: - unix_sockets = set_free_free(unix_sockets); + unix_sockets = TAKE_PTR(sockets); + return 1; } static bool unix_socket_alive(const char *fn) { assert(fn); - load_unix_sockets(); + if (load_unix_sockets() < 0) + return true; /* We don't know, so assume yes */ - if (unix_sockets) - return !!set_get(unix_sockets, (char*) fn); - - /* We don't know, so assume yes */ - return true; + return !!set_get(unix_sockets, (char*) fn); } static int dir_is_mount_point(DIR *d, const char *subdir) { From 87938c3bea13fe1a237303fd15c69a0249a96c18 Mon Sep 17 00:00:00 2001 From: Yu Watanabe Date: Mon, 26 Nov 2018 01:00:54 +0900 Subject: [PATCH 14/21] tmpfiles: define main through macro --- src/tmpfiles/tmpfiles.c | 90 ++++++++++++++++++----------------------- 1 file changed, 40 insertions(+), 50 deletions(-) diff --git a/src/tmpfiles/tmpfiles.c b/src/tmpfiles/tmpfiles.c index b673ce1080..9cd317e97b 100644 --- a/src/tmpfiles/tmpfiles.c +++ b/src/tmpfiles/tmpfiles.c @@ -39,6 +39,7 @@ #include "label.h" #include "log.h" #include "macro.h" +#include "main-func.h" #include "missing.h" #include "mkdir.h" #include "mountpoint-util.h" @@ -172,6 +173,13 @@ static char *arg_replace = NULL; static OrderedHashmap *items = NULL, *globs = NULL; static Set *unix_sockets = NULL; +STATIC_DESTRUCTOR_REGISTER(items, ordered_hashmap_freep); +STATIC_DESTRUCTOR_REGISTER(globs, ordered_hashmap_freep); +STATIC_DESTRUCTOR_REGISTER(unix_sockets, set_free_freep); +STATIC_DESTRUCTOR_REGISTER(arg_include_prefixes, freep); +STATIC_DESTRUCTOR_REGISTER(arg_exclude_prefixes, freep); +STATIC_DESTRUCTOR_REGISTER(arg_root, freep); + static int specifier_machine_id_safe(char specifier, void *data, void *userdata, char **ret); static int specifier_directory(char specifier, void *data, void *userdata, char **ret); @@ -2318,18 +2326,18 @@ static void item_free_contents(Item *i) { #endif } -static void item_array_free(ItemArray *a) { +static ItemArray* item_array_free(ItemArray *a) { size_t n; if (!a) - return; + return NULL; for (n = 0; n < a->n_items; n++) item_free_contents(a->items + n); set_free(a->children); free(a->items); - free(a); + return mfree(a); } static int item_compare(const Item *a, const Item *b) { @@ -3149,9 +3157,11 @@ static int link_parent(ItemArray *a) { return 0; } -int main(int argc, char *argv[]) { +DEFINE_PRIVATE_HASH_OPS_WITH_VALUE_DESTRUCTOR(item_array_hash_ops, char, string_hash_func, string_compare_func, + ItemArray, item_array_free); + +static int run(int argc, char *argv[]) { _cleanup_strv_free_ char **config_dirs = NULL; - int r, k, r_process = 0; bool invalid_config = false; Iterator iterator; ItemArray *a; @@ -3160,25 +3170,22 @@ int main(int argc, char *argv[]) { PHASE_CREATE, _PHASE_MAX } phase; + int r, k; r = parse_argv(argc, argv); if (r <= 0) - goto finish; + return r; log_setup_service(); if (arg_user) { r = user_config_paths(&config_dirs); - if (r < 0) { - log_error_errno(r, "Failed to initialize configuration directory list: %m"); - goto finish; - } + if (r < 0) + return log_error_errno(r, "Failed to initialize configuration directory list: %m"); } else { config_dirs = strv_split_nulstr(CONF_PATHS_NULSTR("tmpfiles.d")); - if (!config_dirs) { - r = log_oom(); - goto finish; - } + if (!config_dirs) + return log_oom(); } if (DEBUG_LOGGING) { @@ -3192,21 +3199,17 @@ int main(int argc, char *argv[]) { if (arg_cat_config) { (void) pager_open(arg_pager_flags); - r = cat_config(config_dirs, argv + optind); - goto finish; + return cat_config(config_dirs, argv + optind); } umask(0022); mac_selinux_init(); - items = ordered_hashmap_new(&string_hash_ops); - globs = ordered_hashmap_new(&string_hash_ops); - - if (!items || !globs) { - r = log_oom(); - goto finish; - } + items = ordered_hashmap_new(&item_array_hash_ops); + globs = ordered_hashmap_new(&item_array_hash_ops); + if (!items || !globs) + return log_oom(); /* If command line arguments are specified along with --replace, read all * configuration files and insert the positional arguments at the specified @@ -3219,18 +3222,18 @@ int main(int argc, char *argv[]) { else r = parse_arguments(config_dirs, argv + optind, &invalid_config); if (r < 0) - goto finish; + return r; /* Let's now link up all child/parent relationships */ ORDERED_HASHMAP_FOREACH(a, items, iterator) { r = link_parent(a); if (r < 0) - goto finish; + return r; } ORDERED_HASHMAP_FOREACH(a, globs, iterator) { r = link_parent(a); if (r < 0) - goto finish; + return r; } /* If multiple operations are requested, let's first run the remove/clean operations, and only then the create @@ -3251,38 +3254,25 @@ int main(int argc, char *argv[]) { /* The non-globbing ones usually create things, hence we apply them first */ ORDERED_HASHMAP_FOREACH(a, items, iterator) { k = process_item_array(a, op); - if (k < 0 && r_process == 0) - r_process = k; + if (k < 0 && r >= 0) + r = k; } /* The globbing ones usually alter things, hence we apply them second. */ ORDERED_HASHMAP_FOREACH(a, globs, iterator) { k = process_item_array(a, op); - if (k < 0 && r_process == 0) - r_process = k; + if (k < 0 && r >= 0) + r = k; } } -finish: - pager_close(); - - ordered_hashmap_free_with_destructor(items, item_array_free); - ordered_hashmap_free_with_destructor(globs, item_array_free); - - free(arg_include_prefixes); - free(arg_exclude_prefixes); - free(arg_root); - - set_free_free(unix_sockets); - - mac_selinux_finish(); - - if (r < 0 || ERRNO_IS_RESOURCE(-r_process)) - return EXIT_FAILURE; - else if (invalid_config) + if (ERRNO_IS_RESOURCE(-r)) + return r; + if (invalid_config) return EX_DATAERR; - else if (r_process < 0) + if (r < 0) return EX_CANTCREAT; - else - return EXIT_SUCCESS; + return 0; } + +DEFINE_MAIN_FUNCTION_WITH_POSITIVE_FAILURE(run); From b07ec5a173f285c35661150764562fb78bb6fadd Mon Sep 17 00:00:00 2001 From: Yu Watanabe Date: Wed, 28 Nov 2018 22:54:44 +0900 Subject: [PATCH 15/21] machine-image: introduce image_hash_ops and use it --- src/machine/image-dbus.c | 8 ++++---- src/machine/machined-dbus.c | 8 ++++---- src/machine/machined.c | 3 +-- src/portable/portabled-bus.c | 4 ++-- src/portable/portabled-image-bus.c | 4 ++-- src/portable/portabled-image.c | 4 ++-- src/portable/portabled.c | 2 +- src/shared/machine-image.c | 2 ++ src/shared/machine-image.h | 7 ++----- 9 files changed, 20 insertions(+), 22 deletions(-) diff --git a/src/machine/image-dbus.c b/src/machine/image-dbus.c index 89df274544..a311ed9077 100644 --- a/src/machine/image-dbus.c +++ b/src/machine/image-dbus.c @@ -382,7 +382,7 @@ static int image_flush_cache(sd_event_source *s, void *userdata) { assert(s); assert(m); - hashmap_clear_with_destructor(m->image_cache, image_unref); + hashmap_clear(m->image_cache); return 0; } @@ -412,7 +412,7 @@ int image_object_find(sd_bus *bus, const char *path, const char *interface, void return 1; } - r = hashmap_ensure_allocated(&m->image_cache, &string_hash_ops); + r = hashmap_ensure_allocated(&m->image_cache, &image_hash_ops); if (r < 0) return r; @@ -461,7 +461,7 @@ char *image_bus_path(const char *name) { } int image_node_enumerator(sd_bus *bus, const char *path, void *userdata, char ***nodes, sd_bus_error *error) { - _cleanup_(image_hashmap_freep) Hashmap *images = NULL; + _cleanup_hashmap_free_ Hashmap *images = NULL; _cleanup_strv_free_ char **l = NULL; Image *image; Iterator i; @@ -471,7 +471,7 @@ int image_node_enumerator(sd_bus *bus, const char *path, void *userdata, char ** assert(path); assert(nodes); - images = hashmap_new(&string_hash_ops); + images = hashmap_new(&image_hash_ops); if (!images) return -ENOMEM; diff --git a/src/machine/machined-dbus.c b/src/machine/machined-dbus.c index 453ca30b64..ef1958d8b0 100644 --- a/src/machine/machined-dbus.c +++ b/src/machine/machined-dbus.c @@ -463,7 +463,7 @@ static int method_get_machine_os_release(sd_bus_message *message, void *userdata static int method_list_images(sd_bus_message *message, void *userdata, sd_bus_error *error) { _cleanup_(sd_bus_message_unrefp) sd_bus_message *reply = NULL; - _cleanup_(image_hashmap_freep) Hashmap *images = NULL; + _cleanup_hashmap_free_ Hashmap *images = NULL; Manager *m = userdata; Image *image; Iterator i; @@ -472,7 +472,7 @@ static int method_list_images(sd_bus_message *message, void *userdata, sd_bus_er assert(message); assert(m); - images = hashmap_new(&string_hash_ops); + images = hashmap_new(&image_hash_ops); if (!images) return -ENOMEM; @@ -740,7 +740,7 @@ static int method_clean_pool(sd_bus_message *message, void *userdata, sd_bus_err if (r < 0) return sd_bus_error_set_errnof(error, r, "Failed to fork(): %m"); if (r == 0) { - _cleanup_(image_hashmap_freep) Hashmap *images = NULL; + _cleanup_hashmap_free_ Hashmap *images = NULL; bool success = true; Image *image; Iterator i; @@ -748,7 +748,7 @@ static int method_clean_pool(sd_bus_message *message, void *userdata, sd_bus_err errno_pipe_fd[0] = safe_close(errno_pipe_fd[0]); - images = hashmap_new(&string_hash_ops); + images = hashmap_new(&image_hash_ops); if (!images) { r = -ENOMEM; goto child_fail; diff --git a/src/machine/machined.c b/src/machine/machined.c index 22eb030dee..6742722d0d 100644 --- a/src/machine/machined.c +++ b/src/machine/machined.c @@ -77,8 +77,7 @@ static Manager* manager_unref(Manager *m) { hashmap_free(m->machines); hashmap_free(m->machine_units); hashmap_free(m->machine_leaders); - - hashmap_free_with_destructor(m->image_cache, image_unref); + hashmap_free(m->image_cache); sd_event_source_unref(m->image_cache_defer_event); diff --git a/src/portable/portabled-bus.c b/src/portable/portabled-bus.c index 70fc750fe3..ec93838538 100644 --- a/src/portable/portabled-bus.c +++ b/src/portable/portabled-bus.c @@ -131,7 +131,7 @@ static int method_get_image(sd_bus_message *message, void *userdata, sd_bus_erro static int method_list_images(sd_bus_message *message, void *userdata, sd_bus_error *error) { _cleanup_(sd_bus_message_unrefp) sd_bus_message *reply = NULL; - _cleanup_(image_hashmap_freep) Hashmap *images = NULL; + _cleanup_hashmap_free_ Hashmap *images = NULL; Manager *m = userdata; Image *image; Iterator i; @@ -140,7 +140,7 @@ static int method_list_images(sd_bus_message *message, void *userdata, sd_bus_er assert(message); assert(m); - images = hashmap_new(&string_hash_ops); + images = hashmap_new(&image_hash_ops); if (!images) return -ENOMEM; diff --git a/src/portable/portabled-image-bus.c b/src/portable/portabled-image-bus.c index 0e1f7c5e87..ec39205dc4 100644 --- a/src/portable/portabled-image-bus.c +++ b/src/portable/portabled-image-bus.c @@ -693,7 +693,7 @@ not_found: } int bus_image_node_enumerator(sd_bus *bus, const char *path, void *userdata, char ***nodes, sd_bus_error *error) { - _cleanup_(image_hashmap_freep) Hashmap *images = NULL; + _cleanup_hashmap_free_ Hashmap *images = NULL; _cleanup_strv_free_ char **l = NULL; size_t n_allocated = 0, n = 0; Manager *m = userdata; @@ -705,7 +705,7 @@ int bus_image_node_enumerator(sd_bus *bus, const char *path, void *userdata, cha assert(path); assert(nodes); - images = hashmap_new(&string_hash_ops); + images = hashmap_new(&image_hash_ops); if (!images) return -ENOMEM; diff --git a/src/portable/portabled-image.c b/src/portable/portabled-image.c index a6b3f52e99..d95845b32a 100644 --- a/src/portable/portabled-image.c +++ b/src/portable/portabled-image.c @@ -16,7 +16,7 @@ static int image_cache_flush(sd_event_source *s, void *userdata) { assert(s); assert(m); - hashmap_clear_with_destructor(m->image_cache, image_unref); + hashmap_clear(m->image_cache); return 0; } @@ -25,7 +25,7 @@ static int manager_image_cache_initialize(Manager *m) { assert(m); - r = hashmap_ensure_allocated(&m->image_cache, &string_hash_ops); + r = hashmap_ensure_allocated(&m->image_cache, &image_hash_ops); if (r < 0) return r; diff --git a/src/portable/portabled.c b/src/portable/portabled.c index cfaf2a59e3..63fc340469 100644 --- a/src/portable/portabled.c +++ b/src/portable/portabled.c @@ -47,7 +47,7 @@ static int manager_new(Manager **ret) { static Manager* manager_unref(Manager *m) { assert(m); - hashmap_free_with_destructor(m->image_cache, image_unref); + hashmap_free(m->image_cache); sd_event_source_unref(m->image_cache_defer_event); diff --git a/src/shared/machine-image.c b/src/shared/machine-image.c index d5f6ab8529..28a05976be 100644 --- a/src/shared/machine-image.c +++ b/src/shared/machine-image.c @@ -70,6 +70,8 @@ static Image *image_free(Image *i) { } DEFINE_TRIVIAL_REF_UNREF_FUNC(Image, image, image_free); +DEFINE_HASH_OPS_WITH_VALUE_DESTRUCTOR(image_hash_ops, char, string_hash_func, string_compare_func, + Image, image_unref); static char **image_settings_path(Image *image) { _cleanup_strv_free_ char **l = NULL; diff --git a/src/shared/machine-image.h b/src/shared/machine-image.h index de06147e7b..9fd45899c8 100644 --- a/src/shared/machine-image.h +++ b/src/shared/machine-image.h @@ -59,12 +59,7 @@ typedef struct Image { Image *image_unref(Image *i); Image *image_ref(Image *i); -static inline Hashmap* image_hashmap_free(Hashmap *map) { - return hashmap_free_with_destructor(map, image_unref); -} - DEFINE_TRIVIAL_CLEANUP_FUNC(Image*, image_unref); -DEFINE_TRIVIAL_CLEANUP_FUNC(Hashmap*, image_hashmap_free); int image_find(ImageClass class, const char *name, Image **ret); int image_from_path(const char *path, Image **ret); @@ -113,3 +108,5 @@ static inline bool IMAGE_IS_HOST(const struct Image *i) { return false; } + +extern const struct hash_ops image_hash_ops; From bb1a05d6f34b6f17ac1b6b49c93ec0f8f97dfce7 Mon Sep 17 00:00:00 2001 From: Yu Watanabe Date: Wed, 28 Nov 2018 22:55:13 +0900 Subject: [PATCH 16/21] machine: introduce machine_hash_ops and use it --- src/machine/machine.c | 7 ++++--- src/machine/machine.h | 2 +- src/machine/machined.c | 10 ++++------ 3 files changed, 9 insertions(+), 10 deletions(-) diff --git a/src/machine/machine.c b/src/machine/machine.c index e114541b3f..c76dfc1846 100644 --- a/src/machine/machine.c +++ b/src/machine/machine.c @@ -70,8 +70,9 @@ fail: return mfree(m); } -void machine_free(Machine *m) { - assert(m); +Machine* machine_free(Machine *m) { + if (!m) + return NULL; while (m->operations) operation_free(m->operations); @@ -98,7 +99,7 @@ void machine_free(Machine *m) { free(m->service); free(m->root_directory); free(m->netif); - free(m); + return mfree(m); } int machine_save(Machine *m) { diff --git a/src/machine/machine.h b/src/machine/machine.h index 967c11039b..31527d029b 100644 --- a/src/machine/machine.h +++ b/src/machine/machine.h @@ -65,7 +65,7 @@ struct Machine { }; Machine* machine_new(Manager *manager, MachineClass class, const char *name); -void machine_free(Machine *m); +Machine* machine_free(Machine *m); bool machine_may_gc(Machine *m, bool drop_not_started); void machine_add_to_gc_queue(Machine *m); int machine_start(Machine *m, sd_bus_message *properties, sd_bus_error *error); diff --git a/src/machine/machined.c b/src/machine/machined.c index 6742722d0d..dec2164bb0 100644 --- a/src/machine/machined.c +++ b/src/machine/machined.c @@ -25,6 +25,9 @@ static Manager* manager_unref(Manager *m); DEFINE_TRIVIAL_CLEANUP_FUNC(Manager*, manager_unref); +DEFINE_PRIVATE_HASH_OPS_WITH_VALUE_DESTRUCTOR(machine_hash_ops, void, trivial_hash_func, trivial_compare_func, + Machine, machine_free); + static int manager_new(Manager **ret) { _cleanup_(manager_unrefp) Manager *m = NULL; int r; @@ -37,7 +40,7 @@ static int manager_new(Manager **ret) { m->machines = hashmap_new(&string_hash_ops); m->machine_units = hashmap_new(&string_hash_ops); - m->machine_leaders = hashmap_new(NULL); + m->machine_leaders = hashmap_new(&machine_hash_ops); if (!m->machines || !m->machine_units || !m->machine_leaders) return -ENOMEM; @@ -61,8 +64,6 @@ static int manager_new(Manager **ret) { } static Manager* manager_unref(Manager *m) { - Machine *machine; - if (!m) return NULL; @@ -71,9 +72,6 @@ static Manager* manager_unref(Manager *m) { assert(m->n_operations == 0); - while ((machine = hashmap_first(m->machines))) - machine_free(machine); - hashmap_free(m->machines); hashmap_free(m->machine_units); hashmap_free(m->machine_leaders); From 67818055b7ff196d4b279f5d28e26d1ffce76a17 Mon Sep 17 00:00:00 2001 From: Yu Watanabe Date: Wed, 28 Nov 2018 22:55:40 +0900 Subject: [PATCH 17/21] portable: introduce portable_metadata_hash_ops and use it --- src/portable/portable.c | 28 ++++++++-------------------- src/portable/portable.h | 3 --- src/portable/portabled-image-bus.c | 2 +- 3 files changed, 9 insertions(+), 24 deletions(-) diff --git a/src/portable/portable.c b/src/portable/portable.c index ca8043b41e..fce218a23b 100644 --- a/src/portable/portable.c +++ b/src/portable/portable.c @@ -92,21 +92,6 @@ PortableMetadata *portable_metadata_unref(PortableMetadata *i) { return mfree(i); } -Hashmap *portable_metadata_hashmap_unref(Hashmap *h) { - - for (;;) { - PortableMetadata *i; - - i = hashmap_steal_first(h); - if (!i) - break; - - portable_metadata_unref(i); - } - - return hashmap_free(h); -} - static int compare_metadata(PortableMetadata *const *x, PortableMetadata *const *y) { return strcmp((*x)->name, (*y)->name); } @@ -232,6 +217,9 @@ static int recv_item( return 0; } +DEFINE_PRIVATE_HASH_OPS_WITH_VALUE_DESTRUCTOR(portable_metadata_hash_ops, char, string_hash_func, string_compare_func, + PortableMetadata, portable_metadata_unref); + static int extract_now( const char *where, char **matches, @@ -239,7 +227,7 @@ static int extract_now( PortableMetadata **ret_os_release, Hashmap **ret_unit_files) { - _cleanup_(portable_metadata_hashmap_unrefp) Hashmap *unit_files = NULL; + _cleanup_hashmap_free_ Hashmap *unit_files = NULL; _cleanup_(portable_metadata_unrefp) PortableMetadata *os_release = NULL; _cleanup_(lookup_paths_free) LookupPaths paths = {}; _cleanup_close_ int os_release_fd = -1; @@ -286,7 +274,7 @@ static int extract_now( if (r < 0) return log_debug_errno(r, "Failed to acquire lookup paths: %m"); - unit_files = hashmap_new(&string_hash_ops); + unit_files = hashmap_new(&portable_metadata_hash_ops); if (!unit_files) return -ENOMEM; @@ -362,7 +350,7 @@ static int portable_extract_by_path( Hashmap **ret_unit_files, sd_bus_error *error) { - _cleanup_(portable_metadata_hashmap_unrefp) Hashmap *unit_files = NULL; + _cleanup_hashmap_free_ Hashmap *unit_files = NULL; _cleanup_(portable_metadata_unrefp) PortableMetadata* os_release = NULL; _cleanup_(loop_device_unrefp) LoopDevice *d = NULL; int r; @@ -432,7 +420,7 @@ static int portable_extract_by_path( seq[1] = safe_close(seq[1]); - unit_files = hashmap_new(&string_hash_ops); + unit_files = hashmap_new(&portable_metadata_hash_ops); if (!unit_files) return -ENOMEM; @@ -986,7 +974,7 @@ int portable_attach( size_t *n_changes, sd_bus_error *error) { - _cleanup_(portable_metadata_hashmap_unrefp) Hashmap *unit_files = NULL; + _cleanup_hashmap_free_ Hashmap *unit_files = NULL; _cleanup_(lookup_paths_free) LookupPaths paths = {}; _cleanup_(image_unrefp) Image *image = NULL; PortableMetadata *item; diff --git a/src/portable/portable.h b/src/portable/portable.h index 9fbf61265a..ad81b584a5 100644 --- a/src/portable/portable.h +++ b/src/portable/portable.h @@ -54,9 +54,6 @@ typedef struct PortableChange { PortableMetadata *portable_metadata_unref(PortableMetadata *i); DEFINE_TRIVIAL_CLEANUP_FUNC(PortableMetadata*, portable_metadata_unref); -Hashmap *portable_metadata_hashmap_unref(Hashmap *h); -DEFINE_TRIVIAL_CLEANUP_FUNC(Hashmap*, portable_metadata_hashmap_unref); - int portable_metadata_hashmap_to_sorted_array(Hashmap *unit_files, PortableMetadata ***ret); int portable_extract(const char *image, char **matches, PortableMetadata **ret_os_release, Hashmap **ret_unit_files, sd_bus_error *error); diff --git a/src/portable/portabled-image-bus.c b/src/portable/portabled-image-bus.c index ec39205dc4..77eb2df118 100644 --- a/src/portable/portabled-image-bus.c +++ b/src/portable/portabled-image-bus.c @@ -93,7 +93,7 @@ int bus_image_common_get_metadata( sd_bus_error *error) { _cleanup_(portable_metadata_unrefp) PortableMetadata *os_release = NULL; - _cleanup_(portable_metadata_hashmap_unrefp) Hashmap *unit_files = NULL; + _cleanup_hashmap_free_ Hashmap *unit_files = NULL; _cleanup_(sd_bus_message_unrefp) sd_bus_message *reply = NULL; _cleanup_free_ PortableMetadata **sorted = NULL; _cleanup_strv_free_ char **matches = NULL; From e924c60f69506620f69e4b0cb76b86fa8040107e Mon Sep 17 00:00:00 2001 From: Yu Watanabe Date: Mon, 26 Nov 2018 03:32:20 +0900 Subject: [PATCH 18/21] udev: use hashmap_clear_free_key() and hashmap_free_free_key() --- src/udev/udev-event.c | 6 +----- src/udev/udev-rules.c | 8 ++------ 2 files changed, 3 insertions(+), 11 deletions(-) diff --git a/src/udev/udev-event.c b/src/udev/udev-event.c index 840d20beac..3782d892c2 100644 --- a/src/udev/udev-event.c +++ b/src/udev/udev-event.c @@ -64,17 +64,13 @@ struct udev_event *udev_event_new(sd_device *dev, usec_t exec_delay_usec, sd_net } struct udev_event *udev_event_free(struct udev_event *event) { - void *p; - if (!event) return NULL; sd_device_unref(event->dev); sd_device_unref(event->dev_db_clone); sd_netlink_unref(event->rtnl); - while ((p = hashmap_steal_first_key(event->run_list))) - free(p); - hashmap_free(event->run_list); + hashmap_free_free_key(event->run_list); hashmap_free_free_free(event->seclabel_list); free(event->program_result); free(event->name); diff --git a/src/udev/udev-rules.c b/src/udev/udev-rules.c index f6f640a216..a9fa04dbf0 100644 --- a/src/udev/udev-rules.c +++ b/src/udev/udev-rules.c @@ -2407,12 +2407,8 @@ int udev_rules_apply_to_event( case TK_A_RUN_PROGRAM: { _cleanup_free_ char *cmd = NULL; - if (IN_SET(cur->key.op, OP_ASSIGN, OP_ASSIGN_FINAL)) { - void *p; - - while ((p = hashmap_steal_first_key(event->run_list))) - free(p); - } + if (IN_SET(cur->key.op, OP_ASSIGN, OP_ASSIGN_FINAL)) + hashmap_clear_free_key(event->run_list); r = hashmap_ensure_allocated(&event->run_list, NULL); if (r < 0) From 8276855e48e77e4a7ffe40944df7df5c3db82a73 Mon Sep 17 00:00:00 2001 From: Yu Watanabe Date: Mon, 26 Nov 2018 03:38:48 +0900 Subject: [PATCH 19/21] lldp: rename lldp_neighbor_id_hash_ops and set value destructor --- src/libsystemd-network/lldp-neighbor.c | 3 ++- src/libsystemd-network/lldp-neighbor.h | 2 +- src/libsystemd-network/sd-lldp.c | 7 ++----- 3 files changed, 5 insertions(+), 7 deletions(-) diff --git a/src/libsystemd-network/lldp-neighbor.c b/src/libsystemd-network/lldp-neighbor.c index 023c8e7be6..83b98ba492 100644 --- a/src/libsystemd-network/lldp-neighbor.c +++ b/src/libsystemd-network/lldp-neighbor.c @@ -34,7 +34,8 @@ int lldp_neighbor_id_compare_func(const LLDPNeighborID *x, const LLDPNeighborID return CMP(x->port_id_size, y->port_id_size); } -DEFINE_HASH_OPS(lldp_neighbor_id_hash_ops, LLDPNeighborID, lldp_neighbor_id_hash_func, lldp_neighbor_id_compare_func); +DEFINE_HASH_OPS_WITH_VALUE_DESTRUCTOR(lldp_neighbor_hash_ops, LLDPNeighborID, lldp_neighbor_id_hash_func, lldp_neighbor_id_compare_func, + sd_lldp_neighbor, lldp_neighbor_unlink); int lldp_neighbor_prioq_compare_func(const void *a, const void *b) { const sd_lldp_neighbor *x = a, *y = b; diff --git a/src/libsystemd-network/lldp-neighbor.h b/src/libsystemd-network/lldp-neighbor.h index 2241c3bd9d..62dbff42ca 100644 --- a/src/libsystemd-network/lldp-neighbor.h +++ b/src/libsystemd-network/lldp-neighbor.h @@ -80,7 +80,7 @@ static inline void* LLDP_NEIGHBOR_TLV_DATA(const sd_lldp_neighbor *n) { return ((uint8_t*) LLDP_NEIGHBOR_RAW(n)) + n->rindex + 2; } -extern const struct hash_ops lldp_neighbor_id_hash_ops; +extern const struct hash_ops lldp_neighbor_hash_ops; int lldp_neighbor_id_compare_func(const LLDPNeighborID *x, const LLDPNeighborID *y); int lldp_neighbor_prioq_compare_func(const void *a, const void *b); diff --git a/src/libsystemd-network/sd-lldp.c b/src/libsystemd-network/sd-lldp.c index 4e3ee53cd7..e5cd134d57 100644 --- a/src/libsystemd-network/sd-lldp.c +++ b/src/libsystemd-network/sd-lldp.c @@ -27,12 +27,9 @@ static const char * const lldp_event_table[_SD_LLDP_EVENT_MAX] = { DEFINE_STRING_TABLE_LOOKUP(lldp_event, sd_lldp_event); static void lldp_flush_neighbors(sd_lldp *lldp) { - sd_lldp_neighbor *n; - assert(lldp); - while ((n = hashmap_first(lldp->neighbor_by_id))) - lldp_neighbor_unlink(n); + hashmap_clear(lldp->neighbor_by_id); } static void lldp_callback(sd_lldp *lldp, sd_lldp_event event, sd_lldp_neighbor *n) { @@ -375,7 +372,7 @@ _public_ int sd_lldp_new(sd_lldp **ret) { .capability_mask = (uint16_t) -1, }; - lldp->neighbor_by_id = hashmap_new(&lldp_neighbor_id_hash_ops); + lldp->neighbor_by_id = hashmap_new(&lldp_neighbor_hash_ops); if (!lldp->neighbor_by_id) return -ENOMEM; From f030d36cd0a48856b6970541bd5baf74c2c06449 Mon Sep 17 00:00:00 2001 From: Yu Watanabe Date: Mon, 26 Nov 2018 04:41:53 +0900 Subject: [PATCH 20/21] sysusers: use fgetgrent_sane() --- src/sysusers/sysusers.c | 10 ++-------- 1 file changed, 2 insertions(+), 8 deletions(-) diff --git a/src/sysusers/sysusers.c b/src/sysusers/sysusers.c index c2326ee9b8..3eb70f5e8f 100644 --- a/src/sysusers/sysusers.c +++ b/src/sysusers/sysusers.c @@ -140,8 +140,7 @@ static int load_group_database(void) { if (r < 0) return r; - errno = 0; - while ((gr = fgetgrent(f))) { + while ((r = fgetgrent_sane(f, &gr)) > 0) { char *n; int k, q; @@ -164,13 +163,8 @@ static int load_group_database(void) { if (k <= 0 && q <= 0) free(n); - - errno = 0; } - if (!IN_SET(errno, 0, ENOENT)) - return -errno; - - return 0; + return r; } static int make_backup(const char *target, const char *x) { From 71da167373b79d062f91988d19f83836dc2b7396 Mon Sep 17 00:00:00 2001 From: Yu Watanabe Date: Mon, 26 Nov 2018 04:42:36 +0900 Subject: [PATCH 21/21] sysusers: define main through macro This also fixes possible memleak when uid/gid or user/group name in database are conflicted. --- src/sysusers/sysusers.c | 205 +++++++++++++++++----------------------- 1 file changed, 86 insertions(+), 119 deletions(-) diff --git a/src/sysusers/sysusers.c b/src/sysusers/sysusers.c index 3eb70f5e8f..788cc36d64 100644 --- a/src/sysusers/sysusers.c +++ b/src/sysusers/sysusers.c @@ -12,9 +12,11 @@ #include "format-util.h" #include "fs-util.h" #include "hashmap.h" +#include "main-func.h" #include "pager.h" #include "path-util.h" #include "pretty-print.h" +#include "set.h" #include "selinux-util.h" #include "smack-util.h" #include "specifier.h" @@ -68,13 +70,28 @@ static OrderedHashmap *users = NULL, *groups = NULL; static OrderedHashmap *todo_uids = NULL, *todo_gids = NULL; static OrderedHashmap *members = NULL; -static Hashmap *database_uid = NULL, *database_user = NULL; -static Hashmap *database_gid = NULL, *database_group = NULL; +static Hashmap *database_by_uid = NULL, *database_by_username = NULL; +static Hashmap *database_by_gid = NULL, *database_by_groupname = NULL; +static Set *database_users = NULL, *database_groups = NULL; static uid_t search_uid = UID_INVALID; static UidRange *uid_range = NULL; static unsigned n_uid_range = 0; +STATIC_DESTRUCTOR_REGISTER(groups, ordered_hashmap_freep); +STATIC_DESTRUCTOR_REGISTER(users, ordered_hashmap_freep); +STATIC_DESTRUCTOR_REGISTER(members, ordered_hashmap_freep); +STATIC_DESTRUCTOR_REGISTER(todo_uids, ordered_hashmap_freep); +STATIC_DESTRUCTOR_REGISTER(todo_gids, ordered_hashmap_freep); +STATIC_DESTRUCTOR_REGISTER(database_by_uid, hashmap_freep); +STATIC_DESTRUCTOR_REGISTER(database_by_username, hashmap_freep); +STATIC_DESTRUCTOR_REGISTER(database_users, set_free_freep); +STATIC_DESTRUCTOR_REGISTER(database_by_gid, hashmap_freep); +STATIC_DESTRUCTOR_REGISTER(database_by_groupname, hashmap_freep); +STATIC_DESTRUCTOR_REGISTER(database_groups, set_free_freep); +STATIC_DESTRUCTOR_REGISTER(uid_range, freep); +STATIC_DESTRUCTOR_REGISTER(arg_root, freep); + static int load_user_database(void) { _cleanup_fclose_ FILE *f = NULL; const char *passwd_path; @@ -86,11 +103,15 @@ static int load_user_database(void) { if (!f) return errno == ENOENT ? 0 : -errno; - r = hashmap_ensure_allocated(&database_user, &string_hash_ops); + r = hashmap_ensure_allocated(&database_by_username, &string_hash_ops); if (r < 0) return r; - r = hashmap_ensure_allocated(&database_uid, NULL); + r = hashmap_ensure_allocated(&database_by_uid, NULL); + if (r < 0) + return r; + + r = set_ensure_allocated(&database_users, NULL); if (r < 0) return r; @@ -102,21 +123,19 @@ static int load_user_database(void) { if (!n) return -ENOMEM; - k = hashmap_put(database_user, n, UID_TO_PTR(pw->pw_uid)); - if (k < 0 && k != -EEXIST) { + k = set_put(database_users, n); + if (k < 0) { free(n); return k; } - q = hashmap_put(database_uid, UID_TO_PTR(pw->pw_uid), n); - if (q < 0 && q != -EEXIST) { - if (k <= 0) - free(n); - return q; - } + k = hashmap_put(database_by_username, n, UID_TO_PTR(pw->pw_uid)); + if (k < 0 && k != -EEXIST) + return k; - if (k <= 0 && q <= 0) - free(n); + q = hashmap_put(database_by_uid, UID_TO_PTR(pw->pw_uid), n); + if (q < 0 && q != -EEXIST) + return q; } return r; } @@ -132,11 +151,15 @@ static int load_group_database(void) { if (!f) return errno == ENOENT ? 0 : -errno; - r = hashmap_ensure_allocated(&database_group, &string_hash_ops); + r = hashmap_ensure_allocated(&database_by_groupname, &string_hash_ops); if (r < 0) return r; - r = hashmap_ensure_allocated(&database_gid, NULL); + r = hashmap_ensure_allocated(&database_by_gid, NULL); + if (r < 0) + return r; + + r = set_ensure_allocated(&database_groups, NULL); if (r < 0) return r; @@ -148,21 +171,19 @@ static int load_group_database(void) { if (!n) return -ENOMEM; - k = hashmap_put(database_group, n, GID_TO_PTR(gr->gr_gid)); - if (k < 0 && k != -EEXIST) { + k = set_put(database_groups, n); + if (k < 0) { free(n); return k; } - q = hashmap_put(database_gid, GID_TO_PTR(gr->gr_gid), n); - if (q < 0 && q != -EEXIST) { - if (k <= 0) - free(n); - return q; - } + k = hashmap_put(database_by_groupname, n, GID_TO_PTR(gr->gr_gid)); + if (k < 0 && k != -EEXIST) + return k; - if (k <= 0 && q <= 0) - free(n); + q = hashmap_put(database_by_gid, GID_TO_PTR(gr->gr_gid), n); + if (q < 0 && q != -EEXIST) + return q; } return r; } @@ -816,11 +837,11 @@ static int uid_is_ok(uid_t uid, const char *name, bool check_with_gid) { } /* Let's check the files directly */ - if (hashmap_contains(database_uid, UID_TO_PTR(uid))) + if (hashmap_contains(database_by_uid, UID_TO_PTR(uid))) return 0; if (check_with_gid) { - n = hashmap_get(database_gid, GID_TO_PTR(uid)); + n = hashmap_get(database_by_gid, GID_TO_PTR(uid)); if (n && !streq(n, name)) return 0; } @@ -923,7 +944,7 @@ static int add_user(Item *i) { assert(i); /* Check the database directly */ - z = hashmap_get(database_user, i->name); + z = hashmap_get(database_by_username, i->name); if (z) { log_debug("User %s already exists.", i->name); i->uid = PTR_TO_UID(z); @@ -1040,10 +1061,10 @@ static int gid_is_ok(gid_t gid) { if (ordered_hashmap_get(todo_uids, UID_TO_PTR(gid))) return 0; - if (hashmap_contains(database_gid, GID_TO_PTR(gid))) + if (hashmap_contains(database_by_gid, GID_TO_PTR(gid))) return 0; - if (hashmap_contains(database_uid, UID_TO_PTR(gid))) + if (hashmap_contains(database_by_uid, UID_TO_PTR(gid))) return 0; if (!arg_root) { @@ -1072,7 +1093,7 @@ static int add_group(Item *i) { assert(i); /* Check the database directly */ - z = hashmap_get(database_group, i->name); + z = hashmap_get(database_by_groupname, i->name); if (z) { log_debug("Group %s already exists.", i->name); i->gid = PTR_TO_GID(z); @@ -1221,10 +1242,9 @@ static int process_item(Item *i) { } } -static void item_free(Item *i) { - +static Item* item_free(Item *i) { if (!i) - return; + return NULL; free(i->name); free(i->uid_path); @@ -1232,10 +1252,11 @@ static void item_free(Item *i) { free(i->description); free(i->home); free(i->shell); - free(i); + return mfree(i); } DEFINE_TRIVIAL_CLEANUP_FUNC(Item*, item_free); +DEFINE_PRIVATE_HASH_OPS_WITH_VALUE_DESTRUCTOR(item_hash_ops, char, string_hash_func, string_compare_func, Item, item_free); static int add_implicit(void) { char *g, **l; @@ -1250,7 +1271,7 @@ static int add_implicit(void) { if (!ordered_hashmap_get(users, *m)) { _cleanup_(item_freep) Item *j = NULL; - r = ordered_hashmap_ensure_allocated(&users, &string_hash_ops); + r = ordered_hashmap_ensure_allocated(&users, &item_hash_ops); if (r < 0) return log_oom(); @@ -1275,7 +1296,7 @@ static int add_implicit(void) { ordered_hashmap_get(groups, g))) { _cleanup_(item_freep) Item *j = NULL; - r = ordered_hashmap_ensure_allocated(&groups, &string_hash_ops); + r = ordered_hashmap_ensure_allocated(&groups, &item_hash_ops); if (r < 0) return log_oom(); @@ -1340,6 +1361,8 @@ static bool item_equal(Item *a, Item *b) { return true; } +DEFINE_PRIVATE_HASH_OPS_FULL(members_hash_ops, char, string_hash_func, string_compare_func, free, char*, strv_free); + static int parse_line(const char *fname, unsigned line, const char *buffer) { static const Specifier specifier_table[] = { @@ -1530,7 +1553,7 @@ static int parse_line(const char *fname, unsigned line, const char *buffer) { return -EINVAL; } - r = ordered_hashmap_ensure_allocated(&members, &string_hash_ops); + r = ordered_hashmap_ensure_allocated(&members, &members_hash_ops); if (r < 0) return log_oom(); @@ -1572,7 +1595,7 @@ static int parse_line(const char *fname, unsigned line, const char *buffer) { return -EINVAL; } - r = ordered_hashmap_ensure_allocated(&users, &string_hash_ops); + r = ordered_hashmap_ensure_allocated(&users, &item_hash_ops); if (r < 0) return log_oom(); @@ -1623,7 +1646,7 @@ static int parse_line(const char *fname, unsigned line, const char *buffer) { return -EINVAL; } - r = ordered_hashmap_ensure_allocated(&groups, &string_hash_ops); + r = ordered_hashmap_ensure_allocated(&groups, &item_hash_ops); if (r < 0) return log_oom(); @@ -1725,27 +1748,6 @@ static int read_config_file(const char *fn, bool ignore_enoent) { return r; } -static void free_database(Hashmap *by_name, Hashmap *by_id) { - char *name; - - for (;;) { - name = hashmap_first(by_id); - if (!name) - break; - - hashmap_remove(by_name, name); - - hashmap_steal_first_key(by_id); - free(name); - } - - while ((name = hashmap_steal_first_key(by_name))) - free(name); - - hashmap_free(by_name); - hashmap_free(by_id); -} - static int cat_config(void) { _cleanup_strv_free_ char **files = NULL; int r; @@ -1913,31 +1915,26 @@ static int read_config_files(char **args) { return 0; } -int main(int argc, char *argv[]) { +static int run(int argc, char *argv[]) { _cleanup_close_ int lock = -1; Iterator iterator; - char *n, **v; Item *i; int r; r = parse_argv(argc, argv); if (r <= 0) - goto finish; + return r; log_setup_service(); - if (arg_cat_config) { - r = cat_config(); - goto finish; - } + if (arg_cat_config) + return cat_config(); umask(0022); r = mac_selinux_init(); - if (r < 0) { - log_error_errno(r, "SELinux setup failed: %m"); - goto finish; - } + if (r < 0) + return log_error_errno(r, "SELinux setup failed: %m"); /* If command line arguments are specified along with --replace, read all * configuration files and insert the positional arguments at the specified @@ -1950,48 +1947,38 @@ int main(int argc, char *argv[]) { else r = parse_arguments(argv + optind); if (r < 0) - goto finish; + return r; /* Let's tell nss-systemd not to synthesize the "root" and "nobody" entries for it, so that our detection * whether the names or UID/GID area already used otherwise doesn't get confused. After all, even though * nss-systemd synthesizes these users/groups, they should still appear in /etc/passwd and /etc/group, as the * synthesizing logic is merely supposed to be fallback for cases where we run with a completely unpopulated * /etc. */ - if (setenv("SYSTEMD_NSS_BYPASS_SYNTHETIC", "1", 1) < 0) { - r = log_error_errno(errno, "Failed to set SYSTEMD_NSS_BYPASS_SYNTHETIC environment variable: %m"); - goto finish; - } + if (setenv("SYSTEMD_NSS_BYPASS_SYNTHETIC", "1", 1) < 0) + return log_error_errno(errno, "Failed to set SYSTEMD_NSS_BYPASS_SYNTHETIC environment variable: %m"); if (!uid_range) { /* Default to default range of 1..SYSTEM_UID_MAX */ r = uid_range_add(&uid_range, &n_uid_range, 1, SYSTEM_UID_MAX); - if (r < 0) { - log_oom(); - goto finish; - } + if (r < 0) + return log_oom(); } r = add_implicit(); if (r < 0) - goto finish; + return r; lock = take_etc_passwd_lock(arg_root); - if (lock < 0) { - log_error_errno(lock, "Failed to take /etc/passwd lock: %m"); - goto finish; - } + if (lock < 0) + return log_error_errno(lock, "Failed to take /etc/passwd lock: %m"); r = load_user_database(); - if (r < 0) { - log_error_errno(r, "Failed to load user database: %m"); - goto finish; - } + if (r < 0) + return log_error_errno(r, "Failed to load user database: %m"); r = load_group_database(); - if (r < 0) { - log_error_errno(r, "Failed to read group database: %m"); - goto finish; - } + if (r < 0) + return log_error_errno(r, "Failed to read group database: %m"); ORDERED_HASHMAP_FOREACH(i, groups, iterator) (void) process_item(i); @@ -2001,29 +1988,9 @@ int main(int argc, char *argv[]) { r = write_files(); if (r < 0) - log_error_errno(r, "Failed to write files: %m"); + return log_error_errno(r, "Failed to write files: %m"); -finish: - pager_close(); - - ordered_hashmap_free_with_destructor(groups, item_free); - ordered_hashmap_free_with_destructor(users, item_free); - - while ((v = ordered_hashmap_steal_first_key_and_value(members, (void **) &n))) { - strv_free(v); - free(n); - } - ordered_hashmap_free(members); - - ordered_hashmap_free(todo_uids); - ordered_hashmap_free(todo_gids); - - free_database(database_user, database_uid); - free_database(database_group, database_gid); - - free(uid_range); - - free(arg_root); - - return r < 0 ? EXIT_FAILURE : EXIT_SUCCESS; + return 0; } + +DEFINE_MAIN_FUNCTION(run);