From a16d732a51f93853f2595ead7c14632646610d75 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Zbigniew=20J=C4=99drzejewski-Szmek?= Date: Tue, 16 Feb 2021 08:48:37 +0100 Subject: [PATCH 01/13] fuzz-systemctl-parse-argv: avoid "leak" of bus object Memory sanitizer would report leaked memory from --boot-load-entry=help. Maybe we should disable all bus connections from the fuzzer? It seems not appropriate to communicate with logind. OTOH, in a real fuzzing environment this call should just fail, so maybe that's OK. --- src/systemctl/fuzz-systemctl-parse-argv.c | 3 +++ 1 file changed, 3 insertions(+) diff --git a/src/systemctl/fuzz-systemctl-parse-argv.c b/src/systemctl/fuzz-systemctl-parse-argv.c index f884917e17..cf66fa4bb7 100644 --- a/src/systemctl/fuzz-systemctl-parse-argv.c +++ b/src/systemctl/fuzz-systemctl-parse-argv.c @@ -9,6 +9,7 @@ #include "stdio-util.h" #include "strv.h" #include "systemctl.h" +#include "systemctl-util.h" int LLVMFuzzerTestOneInput(const uint8_t *data, size_t size) { _cleanup_strv_free_ char **argv = NULL; @@ -54,5 +55,7 @@ int LLVMFuzzerTestOneInput(const uint8_t *data, size_t size) { assert_se(freopen(path, "w", stdout)); } + release_busses(); /* We open the bus for communication with logind. + * It needs to be closed to avoid apparent leaks. */ return 0; } From 6aa601c56c6289b1a0a4a20476e110242ee76357 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Zbigniew=20J=C4=99drzejewski-Szmek?= Date: Tue, 16 Feb 2021 13:36:59 +0100 Subject: [PATCH 02/13] analyze: use typedefs for structs and inline iterator variable decls --- src/analyze/analyze.c | 152 ++++++++++++++++++++---------------------- 1 file changed, 73 insertions(+), 79 deletions(-) diff --git a/src/analyze/analyze.c b/src/analyze/analyze.c index 72ebd25525..5ab920f4df 100644 --- a/src/analyze/analyze.c +++ b/src/analyze/analyze.c @@ -92,7 +92,7 @@ static usec_t arg_base_time = USEC_INFINITY; STATIC_DESTRUCTOR_REGISTER(arg_dot_from_patterns, strv_freep); STATIC_DESTRUCTOR_REGISTER(arg_dot_to_patterns, strv_freep); -struct boot_times { +typedef struct BootTimes { usec_t firmware_time; usec_t loader_time; usec_t kernel_time; @@ -124,9 +124,9 @@ struct boot_times { * (so it is stored here for reference). */ usec_t reverse_offset; -}; +} BootTimes; -struct unit_times { +typedef struct UnitTimes { bool has_data; char *name; usec_t activating; @@ -134,9 +134,9 @@ struct unit_times { usec_t deactivated; usec_t deactivating; usec_t time; -}; +} UnitTimes; -struct host_info { +typedef struct HostInfo { char *hostname; char *kernel_name; char *kernel_release; @@ -144,7 +144,7 @@ struct host_info { char *os_pretty_name; char *virtualization; char *architecture; -}; +} HostInfo; static int acquire_bus(sd_bus **bus, bool *use_full_bus) { bool user = arg_scope != UNIT_FILE_SYSTEM; @@ -209,19 +209,16 @@ static int bus_get_unit_property_strv(sd_bus *bus, const char *path, const char return 0; } -static int compare_unit_start(const struct unit_times *a, const struct unit_times *b) { +static int compare_unit_start(const UnitTimes *a, const UnitTimes *b) { return CMP(a->activating, b->activating); } -static void unit_times_free(struct unit_times *t) { - struct unit_times *p; - - for (p = t; p->has_data; p++) +static void unit_times_free(UnitTimes *t) { + for (UnitTimes *p = t; p->has_data; p++) free(p->name); free(t); } - -DEFINE_TRIVIAL_CLEANUP_FUNC(struct unit_times *, unit_times_free); +DEFINE_TRIVIAL_CLEANUP_FUNC(UnitTimes *, unit_times_free); static void subtract_timestamp(usec_t *a, usec_t b) { assert(a); @@ -232,30 +229,30 @@ static void subtract_timestamp(usec_t *a, usec_t b) { } } -static int acquire_boot_times(sd_bus *bus, struct boot_times **bt) { +static int acquire_boot_times(sd_bus *bus, BootTimes **bt) { static const struct bus_properties_map property_map[] = { - { "FirmwareTimestampMonotonic", "t", NULL, offsetof(struct boot_times, firmware_time) }, - { "LoaderTimestampMonotonic", "t", NULL, offsetof(struct boot_times, loader_time) }, - { "KernelTimestamp", "t", NULL, offsetof(struct boot_times, kernel_time) }, - { "InitRDTimestampMonotonic", "t", NULL, offsetof(struct boot_times, initrd_time) }, - { "UserspaceTimestampMonotonic", "t", NULL, offsetof(struct boot_times, userspace_time) }, - { "FinishTimestampMonotonic", "t", NULL, offsetof(struct boot_times, finish_time) }, - { "SecurityStartTimestampMonotonic", "t", NULL, offsetof(struct boot_times, security_start_time) }, - { "SecurityFinishTimestampMonotonic", "t", NULL, offsetof(struct boot_times, security_finish_time) }, - { "GeneratorsStartTimestampMonotonic", "t", NULL, offsetof(struct boot_times, generators_start_time) }, - { "GeneratorsFinishTimestampMonotonic", "t", NULL, offsetof(struct boot_times, generators_finish_time) }, - { "UnitsLoadStartTimestampMonotonic", "t", NULL, offsetof(struct boot_times, unitsload_start_time) }, - { "UnitsLoadFinishTimestampMonotonic", "t", NULL, offsetof(struct boot_times, unitsload_finish_time) }, - { "InitRDSecurityStartTimestampMonotonic", "t", NULL, offsetof(struct boot_times, initrd_security_start_time) }, - { "InitRDSecurityFinishTimestampMonotonic", "t", NULL, offsetof(struct boot_times, initrd_security_finish_time) }, - { "InitRDGeneratorsStartTimestampMonotonic", "t", NULL, offsetof(struct boot_times, initrd_generators_start_time) }, - { "InitRDGeneratorsFinishTimestampMonotonic", "t", NULL, offsetof(struct boot_times, initrd_generators_finish_time) }, - { "InitRDUnitsLoadStartTimestampMonotonic", "t", NULL, offsetof(struct boot_times, initrd_unitsload_start_time) }, - { "InitRDUnitsLoadFinishTimestampMonotonic", "t", NULL, offsetof(struct boot_times, initrd_unitsload_finish_time) }, + { "FirmwareTimestampMonotonic", "t", NULL, offsetof(BootTimes, firmware_time) }, + { "LoaderTimestampMonotonic", "t", NULL, offsetof(BootTimes, loader_time) }, + { "KernelTimestamp", "t", NULL, offsetof(BootTimes, kernel_time) }, + { "InitRDTimestampMonotonic", "t", NULL, offsetof(BootTimes, initrd_time) }, + { "UserspaceTimestampMonotonic", "t", NULL, offsetof(BootTimes, userspace_time) }, + { "FinishTimestampMonotonic", "t", NULL, offsetof(BootTimes, finish_time) }, + { "SecurityStartTimestampMonotonic", "t", NULL, offsetof(BootTimes, security_start_time) }, + { "SecurityFinishTimestampMonotonic", "t", NULL, offsetof(BootTimes, security_finish_time) }, + { "GeneratorsStartTimestampMonotonic", "t", NULL, offsetof(BootTimes, generators_start_time) }, + { "GeneratorsFinishTimestampMonotonic", "t", NULL, offsetof(BootTimes, generators_finish_time) }, + { "UnitsLoadStartTimestampMonotonic", "t", NULL, offsetof(BootTimes, unitsload_start_time) }, + { "UnitsLoadFinishTimestampMonotonic", "t", NULL, offsetof(BootTimes, unitsload_finish_time) }, + { "InitRDSecurityStartTimestampMonotonic", "t", NULL, offsetof(BootTimes, initrd_security_start_time) }, + { "InitRDSecurityFinishTimestampMonotonic", "t", NULL, offsetof(BootTimes, initrd_security_finish_time) }, + { "InitRDGeneratorsStartTimestampMonotonic", "t", NULL, offsetof(BootTimes, initrd_generators_start_time) }, + { "InitRDGeneratorsFinishTimestampMonotonic", "t", NULL, offsetof(BootTimes, initrd_generators_finish_time) }, + { "InitRDUnitsLoadStartTimestampMonotonic", "t", NULL, offsetof(BootTimes, initrd_unitsload_start_time) }, + { "InitRDUnitsLoadFinishTimestampMonotonic", "t", NULL, offsetof(BootTimes, initrd_unitsload_finish_time) }, {}, }; _cleanup_(sd_bus_error_free) sd_bus_error error = SD_BUS_ERROR_NULL; - static struct boot_times times; + static BootTimes times; static bool cached = false; int r; @@ -293,7 +290,7 @@ static int acquire_boot_times(sd_bus *bus, struct boot_times **bt) { } else { /* * User-instance-specific or container-system-specific timestamps processing - * (see comment to reverse_offset in struct boot_times). + * (see comment to reverse_offset in BootTimes). */ times.reverse_offset = times.userspace_time; @@ -316,7 +313,7 @@ finish: return 0; } -static void free_host_info(struct host_info *hi) { +static void free_host_info(HostInfo *hi) { if (!hi) return; @@ -330,20 +327,20 @@ static void free_host_info(struct host_info *hi) { free(hi); } -DEFINE_TRIVIAL_CLEANUP_FUNC(struct host_info *, free_host_info); +DEFINE_TRIVIAL_CLEANUP_FUNC(HostInfo *, free_host_info); -static int acquire_time_data(sd_bus *bus, struct unit_times **out) { +static int acquire_time_data(sd_bus *bus, UnitTimes **out) { static const struct bus_properties_map property_map[] = { - { "InactiveExitTimestampMonotonic", "t", NULL, offsetof(struct unit_times, activating) }, - { "ActiveEnterTimestampMonotonic", "t", NULL, offsetof(struct unit_times, activated) }, - { "ActiveExitTimestampMonotonic", "t", NULL, offsetof(struct unit_times, deactivating) }, - { "InactiveEnterTimestampMonotonic", "t", NULL, offsetof(struct unit_times, deactivated) }, + { "InactiveExitTimestampMonotonic", "t", NULL, offsetof(UnitTimes, activating) }, + { "ActiveEnterTimestampMonotonic", "t", NULL, offsetof(UnitTimes, activated) }, + { "ActiveExitTimestampMonotonic", "t", NULL, offsetof(UnitTimes, deactivating) }, + { "InactiveEnterTimestampMonotonic", "t", NULL, offsetof(UnitTimes, deactivated) }, {}, }; _cleanup_(sd_bus_message_unrefp) sd_bus_message *reply = NULL; _cleanup_(sd_bus_error_free) sd_bus_error error = SD_BUS_ERROR_NULL; - _cleanup_(unit_times_freep) struct unit_times *unit_times = NULL; - struct boot_times *boot_times = NULL; + _cleanup_(unit_times_freep) UnitTimes *unit_times = NULL; + BootTimes *boot_times = NULL; size_t allocated = 0, c = 0; UnitInfo u; int r; @@ -361,7 +358,7 @@ static int acquire_time_data(sd_bus *bus, struct unit_times **out) { return bus_log_parse_error(r); while ((r = bus_parse_unit_info(reply, &u)) > 0) { - struct unit_times *t; + UnitTimes *t; if (!GREEDY_REALLOC(unit_times, allocated, c + 2)) return log_oom(); @@ -414,28 +411,28 @@ static int acquire_time_data(sd_bus *bus, struct unit_times **out) { return c; } -static int acquire_host_info(sd_bus *bus, struct host_info **hi) { +static int acquire_host_info(sd_bus *bus, HostInfo **hi) { static const struct bus_properties_map hostname_map[] = { - { "Hostname", "s", NULL, offsetof(struct host_info, hostname) }, - { "KernelName", "s", NULL, offsetof(struct host_info, kernel_name) }, - { "KernelRelease", "s", NULL, offsetof(struct host_info, kernel_release) }, - { "KernelVersion", "s", NULL, offsetof(struct host_info, kernel_version) }, - { "OperatingSystemPrettyName", "s", NULL, offsetof(struct host_info, os_pretty_name) }, + { "Hostname", "s", NULL, offsetof(HostInfo, hostname) }, + { "KernelName", "s", NULL, offsetof(HostInfo, kernel_name) }, + { "KernelRelease", "s", NULL, offsetof(HostInfo, kernel_release) }, + { "KernelVersion", "s", NULL, offsetof(HostInfo, kernel_version) }, + { "OperatingSystemPrettyName", "s", NULL, offsetof(HostInfo, os_pretty_name) }, {} }; static const struct bus_properties_map manager_map[] = { - { "Virtualization", "s", NULL, offsetof(struct host_info, virtualization) }, - { "Architecture", "s", NULL, offsetof(struct host_info, architecture) }, + { "Virtualization", "s", NULL, offsetof(HostInfo, virtualization) }, + { "Architecture", "s", NULL, offsetof(HostInfo, architecture) }, {} }; _cleanup_(sd_bus_error_free) sd_bus_error error = SD_BUS_ERROR_NULL; _cleanup_(sd_bus_flush_close_unrefp) sd_bus *system_bus = NULL; - _cleanup_(free_host_infop) struct host_info *host; + _cleanup_(free_host_infop) HostInfo *host; int r; - host = new0(struct host_info, 1); + host = new0(HostInfo, 1); if (!host) return log_oom(); @@ -482,7 +479,7 @@ manager: static int pretty_boot_time(sd_bus *bus, char **_buf) { char ts[FORMAT_TIMESPAN_MAX]; - struct boot_times *t; + BootTimes *t; static char buf[4096]; size_t size; char *ptr; @@ -559,14 +556,12 @@ static int pretty_boot_time(sd_bus *bus, char **_buf) { } static void svg_graph_box(double height, double begin, double end) { - long long i; - /* outside box, fill */ svg("\n", SCALE_X * (end - begin), SCALE_Y * height); - for (i = ((long long) (begin / 100000)) * 100000; i <= end; i += 100000) { + for (long long i = ((long long) (begin / 100000)) * 100000; i <= end; i += 100000) { /* lines for each second */ if (i % 5000000 == 0) svg(" \n" @@ -594,7 +589,7 @@ static void svg_graph_box(double height, double begin, double end) { } } -static int plot_unit_times(struct unit_times *u, double width, int y) { +static int plot_unit_times(UnitTimes *u, double width, int y) { char ts[FORMAT_TIMESPAN_MAX]; bool b; @@ -617,13 +612,13 @@ static int plot_unit_times(struct unit_times *u, double width, int y) { } static int analyze_plot(int argc, char *argv[], void *userdata) { - _cleanup_(free_host_infop) struct host_info *host = NULL; + _cleanup_(free_host_infop) HostInfo *host = NULL; _cleanup_(sd_bus_flush_close_unrefp) sd_bus *bus = NULL; - _cleanup_(unit_times_freep) struct unit_times *times = NULL; + _cleanup_(unit_times_freep) UnitTimes *times = NULL; _cleanup_free_ char *pretty_times = NULL; bool use_full_bus = arg_scope == UNIT_FILE_SYSTEM; - struct boot_times *boot; - struct unit_times *u; + BootTimes *boot; + UnitTimes *u; int n, m = 1, y = 0, r; double width; @@ -837,13 +832,12 @@ static int list_dependencies_print( unsigned level, unsigned branches, bool last, - struct unit_times *times, - struct boot_times *boot) { + UnitTimes *times, + BootTimes *boot) { - unsigned i; char ts[FORMAT_TIMESPAN_MAX], ts2[FORMAT_TIMESPAN_MAX]; - for (i = level; i != 0; i--) + for (unsigned i = level; i != 0; i--) printf("%s", special_glyph(branches & (1 << (i-1)) ? SPECIAL_GLYPH_TREE_VERTICAL : SPECIAL_GLYPH_TREE_SPACE)); printf("%s", special_glyph(last ? SPECIAL_GLYPH_TREE_RIGHT : SPECIAL_GLYPH_TREE_BRANCH)); @@ -882,7 +876,7 @@ static Hashmap *unit_times_hashmap; static int list_dependencies_compare(char *const *a, char *const *b) { usec_t usa = 0, usb = 0; - struct unit_times *times; + UnitTimes *times; times = hashmap_get(unit_times_hashmap, *a); if (times) @@ -894,7 +888,7 @@ static int list_dependencies_compare(char *const *a, char *const *b) { return CMP(usb, usa); } -static bool times_in_range(const struct unit_times *times, const struct boot_times *boot) { +static bool times_in_range(const UnitTimes *times, const BootTimes *boot) { return times && times->activated > 0 && times->activated <= boot->finish_time; } @@ -904,8 +898,8 @@ static int list_dependencies_one(sd_bus *bus, const char *name, unsigned level, int r; usec_t service_longest = 0; int to_print = 0; - struct unit_times *times; - struct boot_times *boot; + UnitTimes *times; + BootTimes *boot; if (strv_extend(units, name)) return log_oom(); @@ -970,13 +964,13 @@ static int list_dependencies_one(sd_bus *bus, const char *name, unsigned level, static int list_dependencies(sd_bus *bus, const char *name) { _cleanup_strv_free_ char **units = NULL; char ts[FORMAT_TIMESPAN_MAX]; - struct unit_times *times; + UnitTimes *times; int r; const char *id; _cleanup_free_ char *path = NULL; _cleanup_(sd_bus_message_unrefp) sd_bus_message *reply = NULL; _cleanup_(sd_bus_error_free) sd_bus_error error = SD_BUS_ERROR_NULL; - struct boot_times *boot; + BootTimes *boot; assert(bus); @@ -1021,7 +1015,7 @@ static int list_dependencies(sd_bus *bus, const char *name) { static int analyze_critical_chain(int argc, char *argv[], void *userdata) { _cleanup_(sd_bus_flush_close_unrefp) sd_bus *bus = NULL; - _cleanup_(unit_times_freep) struct unit_times *times = NULL; + _cleanup_(unit_times_freep) UnitTimes *times = NULL; Hashmap *h; int n, r; @@ -1037,7 +1031,7 @@ static int analyze_critical_chain(int argc, char *argv[], void *userdata) { if (!h) return log_oom(); - for (struct unit_times *u = times; u->has_data; u++) { + for (UnitTimes *u = times; u->has_data; u++) { r = hashmap_put(h, u->name, u); if (r < 0) return log_error_errno(r, "Failed to add entry to hashmap: %m"); @@ -1062,7 +1056,7 @@ static int analyze_critical_chain(int argc, char *argv[], void *userdata) { static int analyze_blame(int argc, char *argv[], void *userdata) { _cleanup_(sd_bus_flush_close_unrefp) sd_bus *bus = NULL; - _cleanup_(unit_times_freep) struct unit_times *times = NULL; + _cleanup_(unit_times_freep) UnitTimes *times = NULL; _cleanup_(table_unrefp) Table *table = NULL; TableCell *cell; int n, r; @@ -1103,7 +1097,7 @@ static int analyze_blame(int argc, char *argv[], void *userdata) { if (r < 0) return r; - for (struct unit_times *u = times; u->has_data; u++) { + for (UnitTimes *u = times; u->has_data; u++) { if (u->time <= 0) continue; @@ -1716,7 +1710,7 @@ static int dump_syscall_filters(int argc, char *argv[], void *userdata) { if (strv_isempty(strv_skip(argv, 1))) { _cleanup_set_free_ Set *kernel = NULL, *known = NULL; const char *sys; - int i, k; + int k; NULSTR_FOREACH(sys, syscall_filter_sets[SYSCALL_FILTER_SET_KNOWN].value) if (set_put_strdup(&known, sys) < 0) @@ -1724,7 +1718,7 @@ static int dump_syscall_filters(int argc, char *argv[], void *userdata) { k = load_kernel_syscalls(&kernel); - for (i = 0; i < _SYSCALL_FILTER_SET_MAX; i++) { + for (int i = 0; i < _SYSCALL_FILTER_SET_MAX; i++) { const SyscallFilterSet *set = syscall_filter_sets + i; if (!first) puts(""); From a779cf30d28fb66914b22b3d10f49fa40772c2a8 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Zbigniew=20J=C4=99drzejewski-Szmek?= Date: Tue, 16 Feb 2021 13:50:08 +0100 Subject: [PATCH 03/13] basic/capability-util: add missing initialization There was no error, because the pointer is unconditionally set below. --- src/basic/capability-util.c | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/src/basic/capability-util.c b/src/basic/capability-util.c index c1520d9279..43eb23cb05 100644 --- a/src/basic/capability-util.c +++ b/src/basic/capability-util.c @@ -18,7 +18,7 @@ #include "util.h" int have_effective_cap(int value) { - _cleanup_cap_free_ cap_t cap; + _cleanup_cap_free_ cap_t cap = NULL; cap_flag_value_t fv; cap = cap_get_proc(); From 450918d1111d03732141a2a41a8f95ad1b73e742 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Zbigniew=20J=C4=99drzejewski-Szmek?= Date: Tue, 16 Feb 2021 13:51:58 +0100 Subject: [PATCH 04/13] coredump: add typedef for struct --- src/coredump/coredump-vacuum.c | 17 ++++++++--------- 1 file changed, 8 insertions(+), 9 deletions(-) diff --git a/src/coredump/coredump-vacuum.c b/src/coredump/coredump-vacuum.c index 30c67ffe7c..2b0a2e5250 100644 --- a/src/coredump/coredump-vacuum.c +++ b/src/coredump/coredump-vacuum.c @@ -22,21 +22,20 @@ #define DEFAULT_KEEP_FREE_UPPER (uint64_t) (4ULL*1024ULL*1024ULL*1024ULL) /* 4 GiB */ #define DEFAULT_KEEP_FREE (uint64_t) (1024ULL*1024ULL) /* 1 MB */ -struct vacuum_candidate { +typedef struct VacuumCandidate { unsigned n_files; char *oldest_file; usec_t oldest_mtime; -}; +} VacuumCandidate; -static void vacuum_candidate_free(struct vacuum_candidate *c) { +static void vacuum_candidate_free(VacuumCandidate *c) { if (!c) return; free(c->oldest_file); free(c); } - -DEFINE_TRIVIAL_CLEANUP_FUNC(struct vacuum_candidate*, vacuum_candidate_free); +DEFINE_TRIVIAL_CLEANUP_FUNC(VacuumCandidate*, vacuum_candidate_free); static void vacuum_candidate_hashmap_free(Hashmap *h) { hashmap_free_with_destructor(h, vacuum_candidate_free); @@ -142,14 +141,14 @@ int coredump_vacuum(int exclude_fd, uint64_t keep_free, uint64_t max_use) { for (;;) { _cleanup_(vacuum_candidate_hashmap_freep) Hashmap *h = NULL; - struct vacuum_candidate *worst = NULL; + VacuumCandidate *worst = NULL; struct dirent *de; uint64_t sum = 0; rewinddir(d); FOREACH_DIRENT(de, d, goto fail) { - struct vacuum_candidate *c; + VacuumCandidate *c; struct stat st; uid_t uid; usec_t t; @@ -196,9 +195,9 @@ int coredump_vacuum(int exclude_fd, uint64_t keep_free, uint64_t max_use) { } } else { - _cleanup_(vacuum_candidate_freep) struct vacuum_candidate *n = NULL; + _cleanup_(vacuum_candidate_freep) VacuumCandidate *n = NULL; - n = new0(struct vacuum_candidate, 1); + n = new0(VacuumCandidate, 1); if (!n) return log_oom(); From d3b56a0cad6c397e020b5dc2105cf49ca3d92c42 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Zbigniew=20J=C4=99drzejewski-Szmek?= Date: Tue, 16 Feb 2021 13:57:43 +0100 Subject: [PATCH 05/13] networkd-ndisc: drop confused freepp function The function to cleanup IPv6Token was defined using freep, i.e. the macro generated a freepp function. The correct way would be to do something like #define ipv6_token_free mfree DEFINE_TRIVIAL_CLEANUP_FUNC(IPv6Token *, ipv6_token_free); which would create ipv6_token_freep(). But since the cleanup function is unused, let's just drop it. --- src/network/networkd-ndisc.h | 1 - 1 file changed, 1 deletion(-) diff --git a/src/network/networkd-ndisc.h b/src/network/networkd-ndisc.h index 20884ce56b..8984d88611 100644 --- a/src/network/networkd-ndisc.h +++ b/src/network/networkd-ndisc.h @@ -63,7 +63,6 @@ struct IPv6Token { }; int ipv6token_new(IPv6Token **ret); -DEFINE_TRIVIAL_CLEANUP_FUNC(IPv6Token *, freep); static inline char* NDISC_DNSSL_DOMAIN(const NDiscDNSSL *n) { return ((char*) n) + ALIGN(sizeof(NDiscDNSSL)); From 1f38830153b493b1acc7d1422f5aefd88228d373 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Zbigniew=20J=C4=99drzejewski-Szmek?= Date: Tue, 16 Feb 2021 14:10:28 +0100 Subject: [PATCH 06/13] resolved: make dns_transaction_gc return a pointer _gc() does cleanup if it is possible. So far it returned a bool to signal if it succeeded (false on success). When working on the resolved code I had to look at the definition every time, because the (arguably reversed) calling convention is unobvious. So let's return a pointer (non-NULL: gc has not been done, NULL: gc has been done). This fits nicely with the standard to return a pointer from all free functions obviously. --- src/resolve/resolved-dns-transaction.c | 14 +++++++------- src/resolve/resolved-dns-transaction.h | 2 +- 2 files changed, 8 insertions(+), 8 deletions(-) diff --git a/src/resolve/resolved-dns-transaction.c b/src/resolve/resolved-dns-transaction.c index bd73aa5451..e1ff503929 100644 --- a/src/resolve/resolved-dns-transaction.c +++ b/src/resolve/resolved-dns-transaction.c @@ -141,23 +141,23 @@ DnsTransaction* dns_transaction_free(DnsTransaction *t) { DEFINE_TRIVIAL_CLEANUP_FUNC(DnsTransaction*, dns_transaction_free); -bool dns_transaction_gc(DnsTransaction *t) { +DnsTransaction* dns_transaction_gc(DnsTransaction *t) { assert(t); + /* Returns !NULL if we can't gc yet. */ + if (t->block_gc > 0) - return true; + return t; if (set_isempty(t->notify_query_candidates) && set_isempty(t->notify_query_candidates_done) && set_isempty(t->notify_zone_items) && set_isempty(t->notify_zone_items_done) && set_isempty(t->notify_transactions) && - set_isempty(t->notify_transactions_done)) { - dns_transaction_free(t); - return false; - } + set_isempty(t->notify_transactions_done)) + return dns_transaction_free(t); - return true; + return t; } static uint16_t pick_new_id(Manager *m) { diff --git a/src/resolve/resolved-dns-transaction.h b/src/resolve/resolved-dns-transaction.h index 9376d504bf..3632daeb15 100644 --- a/src/resolve/resolved-dns-transaction.h +++ b/src/resolve/resolved-dns-transaction.h @@ -143,7 +143,7 @@ struct DnsTransaction { int dns_transaction_new(DnsTransaction **ret, DnsScope *s, DnsResourceKey *key, DnsPacket *bypass, uint64_t flags); DnsTransaction* dns_transaction_free(DnsTransaction *t); -bool dns_transaction_gc(DnsTransaction *t); +DnsTransaction* dns_transaction_gc(DnsTransaction *t); DEFINE_TRIVIAL_CLEANUP_FUNC(DnsTransaction*, dns_transaction_gc); int dns_transaction_go(DnsTransaction *t); From 5d160a23041254f58acfd35584103df57dc347ae Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Zbigniew=20J=C4=99drzejewski-Szmek?= Date: Tue, 16 Feb 2021 14:14:41 +0100 Subject: [PATCH 07/13] networkd: make network_config_section_free() inline --- src/network/networkd-util.c | 4 ---- src/network/networkd-util.h | 7 +++++-- 2 files changed, 5 insertions(+), 6 deletions(-) diff --git a/src/network/networkd-util.c b/src/network/networkd-util.c index b41148d2c6..a5c6fa8d00 100644 --- a/src/network/networkd-util.c +++ b/src/network/networkd-util.c @@ -155,10 +155,6 @@ int network_config_section_new(const char *filename, unsigned line, NetworkConfi return 0; } -void network_config_section_free(NetworkConfigSection *cs) { - free(cs); -} - unsigned hashmap_find_free_section_line(Hashmap *hashmap) { NetworkConfigSection *cs; unsigned n = 0; diff --git a/src/network/networkd-util.h b/src/network/networkd-util.h index 0cfb009890..efeeb86eb0 100644 --- a/src/network/networkd-util.h +++ b/src/network/networkd-util.h @@ -47,9 +47,12 @@ sd_dhcp_lease_server_type dhcp_lease_server_type_from_string(const char *s) _pur int kernel_route_expiration_supported(void); -int network_config_section_new(const char *filename, unsigned line, NetworkConfigSection **s); -void network_config_section_free(NetworkConfigSection *network); +static inline void network_config_section_free(NetworkConfigSection *cs) { + free(cs); +} DEFINE_TRIVIAL_CLEANUP_FUNC(NetworkConfigSection*, network_config_section_free); + +int network_config_section_new(const char *filename, unsigned line, NetworkConfigSection **s); extern const struct hash_ops network_config_hash_ops; unsigned hashmap_find_free_section_line(Hashmap *hashmap); From 75db809ae583ef7287e39738195c0825c3070843 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Zbigniew=20J=C4=99drzejewski-Szmek?= Date: Tue, 16 Feb 2021 13:48:04 +0100 Subject: [PATCH 08/13] tree-wide: return NULL from freeing functions I started working on this because I wanted to change how DEFINE_TRIVIAL_CLEANUP_FUNC is defined. Even independently of that change, it's nice to make make things more consistent and predictable. --- src/analyze/analyze.c | 14 +++++++------- src/basic/strbuf.c | 8 ++++---- src/basic/strbuf.h | 4 ++-- src/busctl/busctl.c | 12 +++++------- src/core/automount.c | 6 +++--- src/core/namespace.h | 4 ++-- src/core/socket.c | 6 ++++-- src/core/unit.c | 8 ++++---- src/core/unit.h | 4 ++-- src/coredump/coredump-vacuum.c | 10 +++++----- src/initctl/initctl.c | 7 ++++--- src/journal/journald-stream.c | 6 +++--- src/journal/journald-stream.h | 2 +- src/libsystemd-network/sd-dhcp-server.c | 6 +++--- src/libsystemd/sd-event/sd-event.c | 4 ++-- src/libsystemd/sd-hwdb/hwdb-util.c | 6 +++--- src/login/logind-brightness.c | 6 +++--- src/network/netdev/l2tp-tunnel.c | 8 +++----- src/network/netdev/macsec.c | 18 +++++++++--------- src/network/netdev/wireguard.c | 6 +++--- src/network/networkctl.c | 2 +- src/network/networkd-manager.c | 6 +++--- src/network/networkd-manager.h | 2 +- src/network/networkd-util.h | 7 ++++--- src/network/tc/qdisc.c | 6 +++--- src/network/tc/qdisc.h | 2 +- src/network/tc/tclass.c | 6 +++--- src/network/tc/tclass.h | 2 +- src/network/wait-online/manager.c | 9 +++------ src/network/wait-online/manager.h | 2 +- src/oom/oomd-manager.c | 4 ++-- src/oom/oomd-manager.h | 2 +- src/resolve/resolved-dns-cache.c | 7 +++---- src/resolve/resolved-dns-zone.c | 7 +++---- src/shared/barrier.c | 5 +++-- src/shared/barrier.h | 2 +- src/shared/bitmap.c | 10 +++++----- src/shared/bitmap.h | 6 +++--- src/shared/bus-wait-for-jobs.c | 6 +++--- src/shared/bus-wait-for-jobs.h | 2 +- src/shared/calendarspec.c | 7 +++---- src/shared/dissect-image.c | 6 +++--- src/shared/install.c | 6 +++--- src/systemctl/systemctl-show.c | 7 +++---- src/sysv-generator/sysv-generator.c | 7 +++---- src/timesync/timesyncd-manager.c | 6 +++--- src/timesync/timesyncd-manager.h | 2 +- src/udev/net/link-config.c | 17 ++++++----------- src/udev/net/link-config.h | 2 +- src/udev/udev-ctrl.c | 1 + src/udev/udev-rules.c | 6 +++--- src/udev/udevd.c | 12 ++++++------ 52 files changed, 154 insertions(+), 165 deletions(-) diff --git a/src/analyze/analyze.c b/src/analyze/analyze.c index 5ab920f4df..7271adaf5b 100644 --- a/src/analyze/analyze.c +++ b/src/analyze/analyze.c @@ -213,10 +213,10 @@ static int compare_unit_start(const UnitTimes *a, const UnitTimes *b) { return CMP(a->activating, b->activating); } -static void unit_times_free(UnitTimes *t) { - for (UnitTimes *p = t; p->has_data; p++) +static UnitTimes* unit_times_free(UnitTimes *t) { + for (UnitTimes *p = t; p && p->has_data; p++) free(p->name); - free(t); + return mfree(t); } DEFINE_TRIVIAL_CLEANUP_FUNC(UnitTimes *, unit_times_free); @@ -313,9 +313,9 @@ finish: return 0; } -static void free_host_info(HostInfo *hi) { +static HostInfo* free_host_info(HostInfo *hi) { if (!hi) - return; + return NULL; free(hi->hostname); free(hi->kernel_name); @@ -324,7 +324,7 @@ static void free_host_info(HostInfo *hi) { free(hi->os_pretty_name); free(hi->virtualization); free(hi->architecture); - free(hi); + return mfree(hi); } DEFINE_TRIVIAL_CLEANUP_FUNC(HostInfo *, free_host_info); @@ -429,7 +429,7 @@ static int acquire_host_info(sd_bus *bus, HostInfo **hi) { _cleanup_(sd_bus_error_free) sd_bus_error error = SD_BUS_ERROR_NULL; _cleanup_(sd_bus_flush_close_unrefp) sd_bus *system_bus = NULL; - _cleanup_(free_host_infop) HostInfo *host; + _cleanup_(free_host_infop) HostInfo *host = NULL; int r; host = new0(HostInfo, 1); diff --git a/src/basic/strbuf.c b/src/basic/strbuf.c index aee6647e35..535ac58bed 100644 --- a/src/basic/strbuf.c +++ b/src/basic/strbuf.c @@ -26,7 +26,7 @@ * ... */ -struct strbuf *strbuf_new(void) { +struct strbuf* strbuf_new(void) { struct strbuf *str; str = new(struct strbuf, 1); @@ -65,13 +65,13 @@ void strbuf_complete(struct strbuf *str) { } /* clean up everything */ -void strbuf_cleanup(struct strbuf *str) { +struct strbuf* strbuf_cleanup(struct strbuf *str) { if (!str) - return; + return NULL; strbuf_complete(str); free(str->buf); - free(str); + return mfree(str); } static int strbuf_children_cmp(const struct strbuf_child_entry *n1, diff --git a/src/basic/strbuf.h b/src/basic/strbuf.h index 82758d721b..9a21438d64 100644 --- a/src/basic/strbuf.h +++ b/src/basic/strbuf.h @@ -32,8 +32,8 @@ struct strbuf_child_entry { struct strbuf_node *child; }; -struct strbuf *strbuf_new(void); +struct strbuf* strbuf_new(void); ssize_t strbuf_add_string(struct strbuf *str, const char *s, size_t len); void strbuf_complete(struct strbuf *str); -void strbuf_cleanup(struct strbuf *str); +struct strbuf* strbuf_cleanup(struct strbuf *str); DEFINE_TRIVIAL_CLEANUP_FUNC(struct strbuf*, strbuf_cleanup); diff --git a/src/busctl/busctl.c b/src/busctl/busctl.c index 142aba0b92..fde9240749 100644 --- a/src/busctl/busctl.c +++ b/src/busctl/busctl.c @@ -780,24 +780,22 @@ static int member_compare_funcp(Member * const *a, Member * const *b) { return member_compare_func(*a, *b); } -static void member_free(Member *m) { +static Member* member_free(Member *m) { if (!m) - return; + return NULL; free(m->interface); free(m->name); free(m->signature); free(m->result); free(m->value); - free(m); + return mfree(m); } - DEFINE_TRIVIAL_CLEANUP_FUNC(Member*, member_free); -static void member_set_free(Set *s) { - set_free_with_destructor(s, member_free); +static Set* member_set_free(Set *s) { + return set_free_with_destructor(s, member_free); } - DEFINE_TRIVIAL_CLEANUP_FUNC(Set*, member_set_free); static int on_interface(const char *interface, uint64_t flags, void *userdata) { diff --git a/src/core/automount.c b/src/core/automount.c index 5076b351f5..f0fa5c8ca9 100644 --- a/src/core/automount.c +++ b/src/core/automount.c @@ -48,13 +48,13 @@ struct expire_data { int ioctl_fd; }; -static void expire_data_free(struct expire_data *data) { +static struct expire_data* expire_data_free(struct expire_data *data) { if (!data) - return; + return NULL; safe_close(data->dev_autofs_fd); safe_close(data->ioctl_fd); - free(data); + return mfree(data); } DEFINE_TRIVIAL_CLEANUP_FUNC(struct expire_data*, expire_data_free); diff --git a/src/core/namespace.h b/src/core/namespace.h index a4287869f7..54d4985f80 100644 --- a/src/core/namespace.h +++ b/src/core/namespace.h @@ -137,11 +137,11 @@ int setup_namespace( #define RUN_SYSTEMD_EMPTY "/run/systemd/empty" -static inline void namespace_cleanup_tmpdir(char *p) { +static inline char* namespace_cleanup_tmpdir(char *p) { PROTECT_ERRNO; if (!streq_ptr(p, RUN_SYSTEMD_EMPTY)) (void) rmdir(p); - free(p); + return mfree(p); } DEFINE_TRIVIAL_CLEANUP_FUNC(char*, namespace_cleanup_tmpdir); diff --git a/src/core/socket.c b/src/core/socket.c index cb5316af3a..5631054cce 100644 --- a/src/core/socket.c +++ b/src/core/socket.c @@ -969,6 +969,8 @@ static void socket_close_fds(Socket *s) { if (s->remove_on_stop) STRV_FOREACH(i, s->symlinks) (void) unlink(*i); + + /* Note that we don't return NULL here, since s has not been freed. */ } static void socket_apply_socket_options(Socket *s, SocketPort *p, int fd) { @@ -1611,8 +1613,8 @@ static int socket_address_listen_in_cgroup( DEFINE_TRIVIAL_CLEANUP_FUNC(Socket *, socket_close_fds); -static int socket_open_fds(Socket *_s) { - _cleanup_(socket_close_fdsp) Socket *s = _s; +static int socket_open_fds(Socket *orig_s) { + _cleanup_(socket_close_fdsp) Socket *s = orig_s; _cleanup_(mac_selinux_freep) char *label = NULL; bool know_label = false; SocketPort *p; diff --git a/src/core/unit.c b/src/core/unit.c index 3452712590..e9c22375dd 100644 --- a/src/core/unit.c +++ b/src/core/unit.c @@ -82,7 +82,7 @@ const UnitVTable * const unit_vtable[_UNIT_TYPE_MAX] = { static void maybe_warn_about_dependency(Unit *u, const char *other, UnitDependency dependency); -Unit *unit_new(Manager *m, size_t size) { +Unit* unit_new(Manager *m, size_t size) { Unit *u; assert(m); @@ -607,11 +607,11 @@ static void unit_done(Unit *u) { cgroup_context_done(cc); } -void unit_free(Unit *u) { +Unit* unit_free(Unit *u) { char *t; if (!u) - return; + return NULL; u->transient_file = safe_fclose(u->transient_file); @@ -741,7 +741,7 @@ void unit_free(Unit *u) { set_free_free(u->aliases); free(u->id); - free(u); + return mfree(u); } FreezerState unit_freezer_state(Unit *u) { diff --git a/src/core/unit.h b/src/core/unit.h index 7c13e50878..6d38e66680 100644 --- a/src/core/unit.h +++ b/src/core/unit.h @@ -674,8 +674,8 @@ static inline Unit* UNIT_TRIGGER(Unit *u) { return hashmap_first_key(u->dependencies[UNIT_TRIGGERS]); } -Unit *unit_new(Manager *m, size_t size); -void unit_free(Unit *u); +Unit* unit_new(Manager *m, size_t size); +Unit* unit_free(Unit *u); DEFINE_TRIVIAL_CLEANUP_FUNC(Unit *, unit_free); int unit_new_for_name(Manager *m, size_t size, const char *name, Unit **ret); diff --git a/src/coredump/coredump-vacuum.c b/src/coredump/coredump-vacuum.c index 2b0a2e5250..d20df847e8 100644 --- a/src/coredump/coredump-vacuum.c +++ b/src/coredump/coredump-vacuum.c @@ -28,17 +28,17 @@ typedef struct VacuumCandidate { usec_t oldest_mtime; } VacuumCandidate; -static void vacuum_candidate_free(VacuumCandidate *c) { +static VacuumCandidate* vacuum_candidate_free(VacuumCandidate *c) { if (!c) - return; + return NULL; free(c->oldest_file); - free(c); + return mfree(c); } DEFINE_TRIVIAL_CLEANUP_FUNC(VacuumCandidate*, vacuum_candidate_free); -static void vacuum_candidate_hashmap_free(Hashmap *h) { - hashmap_free_with_destructor(h, vacuum_candidate_free); +static Hashmap* vacuum_candidate_hashmap_free(Hashmap *h) { + return hashmap_free_with_destructor(h, vacuum_candidate_free); } DEFINE_TRIVIAL_CLEANUP_FUNC(Hashmap*, vacuum_candidate_hashmap_free); diff --git a/src/initctl/initctl.c b/src/initctl/initctl.c index d818f452f7..c48fef16ef 100644 --- a/src/initctl/initctl.c +++ b/src/initctl/initctl.c @@ -210,8 +210,9 @@ static int fifo_process(Fifo *f) { return 0; } -static void fifo_free(Fifo *f) { - assert(f); +static Fifo* fifo_free(Fifo *f) { + if (!f) + return NULL; if (f->server) { assert(f->server->n_fifos > 0); @@ -226,7 +227,7 @@ static void fifo_free(Fifo *f) { safe_close(f->fd); } - free(f); + return mfree(f); } DEFINE_TRIVIAL_CLEANUP_FUNC(Fifo*, fifo_free); diff --git a/src/journal/journald-stream.c b/src/journal/journald-stream.c index 1c3652bf5b..73b3240677 100644 --- a/src/journal/journald-stream.c +++ b/src/journal/journald-stream.c @@ -98,9 +98,9 @@ struct StdoutStream { char id_field[STRLEN("_STREAM_ID=") + SD_ID128_STRING_MAX]; }; -void stdout_stream_free(StdoutStream *s) { +StdoutStream* stdout_stream_free(StdoutStream *s) { if (!s) - return; + return NULL; if (s->server) { @@ -129,7 +129,7 @@ void stdout_stream_free(StdoutStream *s) { free(s->state_file); free(s->buffer); - free(s); + return mfree(s); } DEFINE_TRIVIAL_CLEANUP_FUNC(StdoutStream*, stdout_stream_free); diff --git a/src/journal/journald-stream.h b/src/journal/journald-stream.h index 0a033b4dce..7b756c05ec 100644 --- a/src/journal/journald-stream.h +++ b/src/journal/journald-stream.h @@ -9,7 +9,7 @@ typedef struct StdoutStream StdoutStream; int server_open_stdout_socket(Server *s, const char *stdout_socket); int server_restore_streams(Server *s, FDSet *fds); -void stdout_stream_free(StdoutStream *s); +StdoutStream* stdout_stream_free(StdoutStream *s); int stdout_stream_install(Server *s, int fd, StdoutStream **ret); void stdout_stream_destroy(StdoutStream *s); void stdout_stream_send_notify(StdoutStream *s); diff --git a/src/libsystemd-network/sd-dhcp-server.c b/src/libsystemd-network/sd-dhcp-server.c index 80292befeb..bc50664dc7 100644 --- a/src/libsystemd-network/sd-dhcp-server.c +++ b/src/libsystemd-network/sd-dhcp-server.c @@ -649,12 +649,12 @@ static int parse_request(uint8_t code, uint8_t len, const void *option, void *us return 0; } -static void dhcp_request_free(DHCPRequest *req) { +static DHCPRequest* dhcp_request_free(DHCPRequest *req) { if (!req) - return; + return NULL; free(req->client_id.data); - free(req); + return mfree(req); } DEFINE_TRIVIAL_CLEANUP_FUNC(DHCPRequest*, dhcp_request_free); diff --git a/src/libsystemd/sd-event/sd-event.c b/src/libsystemd/sd-event/sd-event.c index 8abbefcf27..d9991cfe19 100644 --- a/src/libsystemd/sd-event/sd-event.c +++ b/src/libsystemd/sd-event/sd-event.c @@ -939,7 +939,7 @@ static void source_disconnect(sd_event_source *s) { sd_event_unref(event); } -static void source_free(sd_event_source *s) { +static sd_event_source* source_free(sd_event_source *s) { assert(s); source_disconnect(s); @@ -989,7 +989,7 @@ static void source_free(sd_event_source *s) { s->destroy_callback(s->userdata); free(s->description); - free(s); + return mfree(s); } DEFINE_TRIVIAL_CLEANUP_FUNC(sd_event_source*, source_free); diff --git a/src/libsystemd/sd-hwdb/hwdb-util.c b/src/libsystemd/sd-hwdb/hwdb-util.c index 4c94ba9c88..34be0e7fcc 100644 --- a/src/libsystemd/sd-hwdb/hwdb-util.c +++ b/src/libsystemd/sd-hwdb/hwdb-util.c @@ -115,13 +115,13 @@ static void trie_node_cleanup(struct trie_node *node) { free(node); } -static void trie_free(struct trie *trie) { +static struct trie* trie_free(struct trie *trie) { if (!trie) - return; + return NULL; trie_node_cleanup(trie->root); strbuf_cleanup(trie->strings); - free(trie); + return mfree(trie); } DEFINE_TRIVIAL_CLEANUP_FUNC(struct trie*, trie_free); diff --git a/src/login/logind-brightness.c b/src/login/logind-brightness.c index f4306749ec..5eb3534f53 100644 --- a/src/login/logind-brightness.c +++ b/src/login/logind-brightness.c @@ -44,9 +44,9 @@ typedef struct BrightnessWriter { sd_event_source* child_event_source; } BrightnessWriter; -static void brightness_writer_free(BrightnessWriter *w) { +static BrightnessWriter* brightness_writer_free(BrightnessWriter *w) { if (!w) - return; + return NULL; if (w->manager && w->path) (void) hashmap_remove_value(w->manager->brightness_writers, w->path, w); @@ -59,7 +59,7 @@ static void brightness_writer_free(BrightnessWriter *w) { w->child_event_source = sd_event_source_unref(w->child_event_source); - free(w); + return mfree(w); } DEFINE_TRIVIAL_CLEANUP_FUNC(BrightnessWriter*, brightness_writer_free); diff --git a/src/network/netdev/l2tp-tunnel.c b/src/network/netdev/l2tp-tunnel.c index e302a2cf3b..a909b15764 100644 --- a/src/network/netdev/l2tp-tunnel.c +++ b/src/network/netdev/l2tp-tunnel.c @@ -39,18 +39,16 @@ static const char* const l2tp_local_address_type_table[_NETDEV_L2TP_LOCAL_ADDRES DEFINE_PRIVATE_STRING_TABLE_LOOKUP_FROM_STRING(l2tp_local_address_type, L2tpLocalAddressType); -static void l2tp_session_free(L2tpSession *s) { +static L2tpSession* l2tp_session_free(L2tpSession *s) { if (!s) - return; + return NULL; if (s->tunnel && s->section) ordered_hashmap_remove(s->tunnel->sessions_by_section, s->section); network_config_section_free(s->section); - free(s->name); - - free(s); + return mfree(s); } DEFINE_NETWORK_SECTION_FUNCTIONS(L2tpSession, l2tp_session_free); diff --git a/src/network/netdev/macsec.c b/src/network/netdev/macsec.c index bef5ba1c51..77c5f8c4e7 100644 --- a/src/network/netdev/macsec.c +++ b/src/network/netdev/macsec.c @@ -35,9 +35,9 @@ static void security_association_init(SecurityAssociation *sa) { sa->use_for_encoding = -1; } -static void macsec_receive_association_free(ReceiveAssociation *c) { +static ReceiveAssociation* macsec_receive_association_free(ReceiveAssociation *c) { if (!c) - return; + return NULL; if (c->macsec && c->section) ordered_hashmap_remove(c->macsec->receive_associations_by_section, c->section); @@ -45,7 +45,7 @@ static void macsec_receive_association_free(ReceiveAssociation *c) { network_config_section_free(c->section); security_association_clear(&c->sa); - free(c); + return mfree(c); } DEFINE_NETWORK_SECTION_FUNCTIONS(ReceiveAssociation, macsec_receive_association_free); @@ -90,9 +90,9 @@ static int macsec_receive_association_new_static(MACsec *s, const char *filename return 0; } -static void macsec_receive_channel_free(ReceiveChannel *c) { +static ReceiveChannel* macsec_receive_channel_free(ReceiveChannel *c) { if (!c) - return; + return NULL; if (c->macsec) { if (c->sci.as_uint64 > 0) @@ -104,7 +104,7 @@ static void macsec_receive_channel_free(ReceiveChannel *c) { network_config_section_free(c->section); - free(c); + return mfree(c); } DEFINE_NETWORK_SECTION_FUNCTIONS(ReceiveChannel, macsec_receive_channel_free); @@ -162,9 +162,9 @@ static int macsec_receive_channel_new_static(MACsec *s, const char *filename, un return 0; } -static void macsec_transmit_association_free(TransmitAssociation *a) { +static TransmitAssociation* macsec_transmit_association_free(TransmitAssociation *a) { if (!a) - return; + return NULL; if (a->macsec && a->section) ordered_hashmap_remove(a->macsec->transmit_associations_by_section, a->section); @@ -172,7 +172,7 @@ static void macsec_transmit_association_free(TransmitAssociation *a) { network_config_section_free(a->section); security_association_clear(&a->sa); - free(a); + return mfree(a); } DEFINE_NETWORK_SECTION_FUNCTIONS(TransmitAssociation, macsec_transmit_association_free); diff --git a/src/network/netdev/wireguard.c b/src/network/netdev/wireguard.c index e442ecc266..75d6d376a5 100644 --- a/src/network/netdev/wireguard.c +++ b/src/network/netdev/wireguard.c @@ -26,11 +26,11 @@ static void resolve_endpoints(NetDev *netdev); -static void wireguard_peer_free(WireguardPeer *peer) { +static WireguardPeer* wireguard_peer_free(WireguardPeer *peer) { WireguardIPmask *mask; if (!peer) - return; + return NULL; if (peer->wireguard) { LIST_REMOVE(peers, peer->wireguard->peers, peer); @@ -54,7 +54,7 @@ static void wireguard_peer_free(WireguardPeer *peer) { free(peer->preshared_key_file); explicit_bzero_safe(peer->preshared_key, WG_KEY_LEN); - free(peer); + return mfree(peer); } DEFINE_NETWORK_SECTION_FUNCTIONS(WireguardPeer, wireguard_peer_free); diff --git a/src/network/networkctl.c b/src/network/networkctl.c index 4381f63a2d..1dd3438abd 100644 --- a/src/network/networkctl.c +++ b/src/network/networkctl.c @@ -230,7 +230,7 @@ static int link_info_compare(const LinkInfo *a, const LinkInfo *b) { return CMP(a->ifindex, b->ifindex); } -static const LinkInfo* link_info_array_free(LinkInfo *array) { +static LinkInfo* link_info_array_free(LinkInfo *array) { for (unsigned i = 0; array && array[i].needs_freeing; i++) { sd_device_unref(array[i].sd_device); free(array[i].ssid); diff --git a/src/network/networkd-manager.c b/src/network/networkd-manager.c index d7e0662453..debd9f1ecc 100644 --- a/src/network/networkd-manager.c +++ b/src/network/networkd-manager.c @@ -850,11 +850,11 @@ int manager_new(Manager **ret) { return 0; } -void manager_free(Manager *m) { +Manager* manager_free(Manager *m) { Link *link; if (!m) - return; + return NULL; free(m->state_file); @@ -909,7 +909,7 @@ void manager_free(Manager *m) { m->fw_ctx = fw_ctx_free(m->fw_ctx); - free(m); + return mfree(m); } int manager_start(Manager *m) { diff --git a/src/network/networkd-manager.h b/src/network/networkd-manager.h index f553abcc54..8b5f8a9dc0 100644 --- a/src/network/networkd-manager.h +++ b/src/network/networkd-manager.h @@ -83,7 +83,7 @@ struct Manager { }; int manager_new(Manager **ret); -void manager_free(Manager *m); +Manager* manager_free(Manager *m); int manager_connect_bus(Manager *m); int manager_start(Manager *m); diff --git a/src/network/networkd-util.h b/src/network/networkd-util.h index efeeb86eb0..1736e2b57b 100644 --- a/src/network/networkd-util.h +++ b/src/network/networkd-util.h @@ -47,8 +47,8 @@ sd_dhcp_lease_server_type dhcp_lease_server_type_from_string(const char *s) _pur int kernel_route_expiration_supported(void); -static inline void network_config_section_free(NetworkConfigSection *cs) { - free(cs); +static inline NetworkConfigSection* network_config_section_free(NetworkConfigSection *cs) { + return mfree(cs); } DEFINE_TRIVIAL_CLEANUP_FUNC(NetworkConfigSection*, network_config_section_free); @@ -66,13 +66,14 @@ static inline bool section_is_invalid(NetworkConfigSection *section) { } #define DEFINE_NETWORK_SECTION_FUNCTIONS(type, free_func) \ - static inline void free_func##_or_set_invalid(type *p) { \ + static inline type* free_func##_or_set_invalid(type *p) { \ assert(p); \ \ if (p->section) \ p->section->invalid = true; \ else \ free_func(p); \ + return NULL; \ } \ DEFINE_TRIVIAL_CLEANUP_FUNC(type*, free_func); \ DEFINE_TRIVIAL_CLEANUP_FUNC(type*, free_func##_or_set_invalid); diff --git a/src/network/tc/qdisc.c b/src/network/tc/qdisc.c index 7016dc1877..2e0f0a12eb 100644 --- a/src/network/tc/qdisc.c +++ b/src/network/tc/qdisc.c @@ -134,9 +134,9 @@ int qdisc_new_static(QDiscKind kind, Network *network, const char *filename, uns return 0; } -void qdisc_free(QDisc *qdisc) { +QDisc* qdisc_free(QDisc *qdisc) { if (!qdisc) - return; + return NULL; if (qdisc->network && qdisc->section) ordered_hashmap_remove(qdisc->network->tc_by_section, qdisc->section); @@ -144,7 +144,7 @@ void qdisc_free(QDisc *qdisc) { network_config_section_free(qdisc->section); free(qdisc->tca_kind); - free(qdisc); + return mfree(qdisc); } static int qdisc_handler(sd_netlink *rtnl, sd_netlink_message *m, Link *link) { diff --git a/src/network/tc/qdisc.h b/src/network/tc/qdisc.h index 2232189f4f..bf2df146a7 100644 --- a/src/network/tc/qdisc.h +++ b/src/network/tc/qdisc.h @@ -74,7 +74,7 @@ extern const QDiscVTable * const qdisc_vtable[_QDISC_KIND_MAX]; /* For casting the various qdisc kinds into a qdisc */ #define QDISC(q) (&(q)->meta) -void qdisc_free(QDisc *qdisc); +QDisc* qdisc_free(QDisc *qdisc); int qdisc_new_static(QDiscKind kind, Network *network, const char *filename, unsigned section_line, QDisc **ret); int qdisc_configure(Link *link, QDisc *qdisc); diff --git a/src/network/tc/tclass.c b/src/network/tc/tclass.c index 64bdfbb90b..d8145997f0 100644 --- a/src/network/tc/tclass.c +++ b/src/network/tc/tclass.c @@ -90,16 +90,16 @@ int tclass_new_static(TClassKind kind, Network *network, const char *filename, u return 0; } -void tclass_free(TClass *tclass) { +TClass* tclass_free(TClass *tclass) { if (!tclass) - return; + return NULL; if (tclass->network && tclass->section) ordered_hashmap_remove(tclass->network->tc_by_section, tclass->section); network_config_section_free(tclass->section); - free(tclass); + return mfree(tclass); } static int tclass_handler(sd_netlink *rtnl, sd_netlink_message *m, Link *link) { diff --git a/src/network/tc/tclass.h b/src/network/tc/tclass.h index 8e5e52c401..fc91789f30 100644 --- a/src/network/tc/tclass.h +++ b/src/network/tc/tclass.h @@ -53,7 +53,7 @@ extern const TClassVTable * const tclass_vtable[_TCLASS_KIND_MAX]; /* For casting the various tclass kinds into a tclass */ #define TCLASS(t) (&(t)->meta) -void tclass_free(TClass *tclass); +TClass* tclass_free(TClass *tclass); int tclass_new_static(TClassKind kind, Network *network, const char *filename, unsigned section_line, TClass **ret); int tclass_configure(Link *link, TClass *tclass); diff --git a/src/network/wait-online/manager.c b/src/network/wait-online/manager.c index 8321395550..f6c8cf909f 100644 --- a/src/network/wait-online/manager.c +++ b/src/network/wait-online/manager.c @@ -347,21 +347,18 @@ int manager_new(Manager **ret, Hashmap *interfaces, char **ignore, return 0; } -void manager_free(Manager *m) { +Manager* manager_free(Manager *m) { if (!m) - return; + return NULL; hashmap_free_with_destructor(m->links, link_free); hashmap_free(m->links_by_name); sd_event_source_unref(m->network_monitor_event_source); sd_network_monitor_unref(m->network_monitor); - sd_event_source_unref(m->rtnl_event_source); sd_netlink_unref(m->rtnl); - sd_event_unref(m->event); - free(m); - return; + return mfree(m); } diff --git a/src/network/wait-online/manager.h b/src/network/wait-online/manager.h index f5e83532ae..9892a43dc9 100644 --- a/src/network/wait-online/manager.h +++ b/src/network/wait-online/manager.h @@ -32,7 +32,7 @@ struct Manager { sd_event *event; }; -void manager_free(Manager *m); +Manager* manager_free(Manager *m); int manager_new(Manager **ret, Hashmap *interfaces, char **ignore, LinkOperationalStateRange required_operstate, bool any, usec_t timeout); diff --git a/src/oom/oomd-manager.c b/src/oom/oomd-manager.c index 94b17dd07b..bdf41807b2 100644 --- a/src/oom/oomd-manager.c +++ b/src/oom/oomd-manager.c @@ -397,7 +397,7 @@ static int monitor_cgroup_contexts(Manager *m) { return 0; } -void manager_free(Manager *m) { +Manager* manager_free(Manager *m) { assert(m); varlink_close_unref(m->varlink); @@ -410,7 +410,7 @@ void manager_free(Manager *m) { hashmap_free(m->monitored_swap_cgroup_contexts); hashmap_free(m->monitored_mem_pressure_cgroup_contexts); - free(m); + return mfree(m); } int manager_new(Manager **ret) { diff --git a/src/oom/oomd-manager.h b/src/oom/oomd-manager.h index 521665e0a8..50f10021c7 100644 --- a/src/oom/oomd-manager.h +++ b/src/oom/oomd-manager.h @@ -51,7 +51,7 @@ struct Manager { Varlink *varlink; }; -void manager_free(Manager *m); +Manager* manager_free(Manager *m); DEFINE_TRIVIAL_CLEANUP_FUNC(Manager*, manager_free); int manager_new(Manager **ret); diff --git a/src/resolve/resolved-dns-cache.c b/src/resolve/resolved-dns-cache.c index cf0e221621..03da73c4ae 100644 --- a/src/resolve/resolved-dns-cache.c +++ b/src/resolve/resolved-dns-cache.c @@ -80,17 +80,16 @@ static const char *dns_cache_item_type_to_string(DnsCacheItem *item) { return NULL; } -static void dns_cache_item_free(DnsCacheItem *i) { +static DnsCacheItem* dns_cache_item_free(DnsCacheItem *i) { if (!i) - return; + return NULL; dns_resource_record_unref(i->rr); dns_resource_key_unref(i->key); dns_answer_unref(i->answer); dns_packet_unref(i->full_packet); - free(i); + return mfree(i); } - DEFINE_TRIVIAL_CLEANUP_FUNC(DnsCacheItem*, dns_cache_item_free); static void dns_cache_item_unlink_and_free(DnsCache *c, DnsCacheItem *i) { diff --git a/src/resolve/resolved-dns-zone.c b/src/resolve/resolved-dns-zone.c index 4c5e1fe372..6f15c53953 100644 --- a/src/resolve/resolved-dns-zone.c +++ b/src/resolve/resolved-dns-zone.c @@ -26,16 +26,15 @@ void dns_zone_item_probe_stop(DnsZoneItem *i) { dns_transaction_gc(t); } -static void dns_zone_item_free(DnsZoneItem *i) { +static DnsZoneItem* dns_zone_item_free(DnsZoneItem *i) { if (!i) - return; + return NULL; dns_zone_item_probe_stop(i); dns_resource_record_unref(i->rr); - free(i); + return mfree(i); } - DEFINE_TRIVIAL_CLEANUP_FUNC(DnsZoneItem*, dns_zone_item_free); static void dns_zone_item_remove_and_free(DnsZone *z, DnsZoneItem *i) { diff --git a/src/shared/barrier.c b/src/shared/barrier.c index 9c93d61a3c..271bba8cde 100644 --- a/src/shared/barrier.c +++ b/src/shared/barrier.c @@ -123,14 +123,15 @@ int barrier_create(Barrier *b) { * * If @b is NULL, this is a no-op. */ -void barrier_destroy(Barrier *b) { +Barrier* barrier_destroy(Barrier *b) { if (!b) - return; + return NULL; b->me = safe_close(b->me); b->them = safe_close(b->them); safe_close_pair(b->pipe); b->barriers = 0; + return NULL; } /** diff --git a/src/shared/barrier.h b/src/shared/barrier.h index b11dce4fc0..b9fc92d752 100644 --- a/src/shared/barrier.h +++ b/src/shared/barrier.h @@ -37,7 +37,7 @@ struct Barrier { #define BARRIER_NULL {-1, -1, {-1, -1}, 0} int barrier_create(Barrier *obj); -void barrier_destroy(Barrier *b); +Barrier* barrier_destroy(Barrier *b); DEFINE_TRIVIAL_CLEANUP_FUNC(Barrier*, barrier_destroy); diff --git a/src/shared/bitmap.c b/src/shared/bitmap.c index 5d450c8832..33528388f0 100644 --- a/src/shared/bitmap.c +++ b/src/shared/bitmap.c @@ -24,11 +24,11 @@ #define BITMAP_NUM_TO_REM(n) ((n) % (sizeof(uint64_t) * 8)) #define BITMAP_OFFSET_TO_NUM(offset, rem) ((offset) * sizeof(uint64_t) * 8 + (rem)) -Bitmap *bitmap_new(void) { +Bitmap* bitmap_new(void) { return new0(Bitmap, 1); } -Bitmap *bitmap_copy(Bitmap *b) { +Bitmap* bitmap_copy(Bitmap *b) { Bitmap *ret; ret = bitmap_new(); @@ -43,12 +43,12 @@ Bitmap *bitmap_copy(Bitmap *b) { return ret; } -void bitmap_free(Bitmap *b) { +Bitmap* bitmap_free(Bitmap *b) { if (!b) - return; + return NULL; free(b->bitmaps); - free(b); + return mfree(b); } int bitmap_ensure_allocated(Bitmap **b) { diff --git a/src/shared/bitmap.h b/src/shared/bitmap.h index 1c305a2c41..5c7651c68d 100644 --- a/src/shared/bitmap.h +++ b/src/shared/bitmap.h @@ -12,10 +12,10 @@ typedef struct Bitmap { size_t bitmaps_allocated; } Bitmap; -Bitmap *bitmap_new(void); -Bitmap *bitmap_copy(Bitmap *b); +Bitmap* bitmap_new(void); +Bitmap* bitmap_copy(Bitmap *b); int bitmap_ensure_allocated(Bitmap **b); -void bitmap_free(Bitmap *b); +Bitmap* bitmap_free(Bitmap *b); int bitmap_set(Bitmap *b, unsigned n); void bitmap_unset(Bitmap *b, unsigned n); diff --git a/src/shared/bus-wait-for-jobs.c b/src/shared/bus-wait-for-jobs.c index 005294c33a..80f7785323 100644 --- a/src/shared/bus-wait-for-jobs.c +++ b/src/shared/bus-wait-for-jobs.c @@ -61,9 +61,9 @@ static int match_job_removed(sd_bus_message *m, void *userdata, sd_bus_error *er return 0; } -void bus_wait_for_jobs_free(BusWaitForJobs *d) { +BusWaitForJobs* bus_wait_for_jobs_free(BusWaitForJobs *d) { if (!d) - return; + return NULL; set_free(d->jobs); @@ -75,7 +75,7 @@ void bus_wait_for_jobs_free(BusWaitForJobs *d) { free(d->name); free(d->result); - free(d); + return mfree(d); } int bus_wait_for_jobs_new(sd_bus *bus, BusWaitForJobs **ret) { diff --git a/src/shared/bus-wait-for-jobs.h b/src/shared/bus-wait-for-jobs.h index 0155887455..68c9d604ad 100644 --- a/src/shared/bus-wait-for-jobs.h +++ b/src/shared/bus-wait-for-jobs.h @@ -8,7 +8,7 @@ typedef struct BusWaitForJobs BusWaitForJobs; int bus_wait_for_jobs_new(sd_bus *bus, BusWaitForJobs **ret); -void bus_wait_for_jobs_free(BusWaitForJobs *d); +BusWaitForJobs* bus_wait_for_jobs_free(BusWaitForJobs *d); int bus_wait_for_jobs_add(BusWaitForJobs *d, const char *path); int bus_wait_for_jobs(BusWaitForJobs *d, bool quiet, const char* const* extra_args); int bus_wait_for_jobs_one(BusWaitForJobs *d, const char *path, bool quiet); diff --git a/src/shared/calendarspec.c b/src/shared/calendarspec.c index c23a91eeed..4f68a570b5 100644 --- a/src/shared/calendarspec.c +++ b/src/shared/calendarspec.c @@ -33,14 +33,13 @@ /* Let's make sure that the microsecond component is safe to be stored in an 'int' */ assert_cc(INT_MAX >= USEC_PER_SEC); -static void chain_free(CalendarComponent *c) { - CalendarComponent *n; - +static CalendarComponent* chain_free(CalendarComponent *c) { while (c) { - n = c->next; + CalendarComponent *n = c->next; free(c); c = n; } + return NULL; } DEFINE_TRIVIAL_CLEANUP_FUNC(CalendarComponent*, chain_free); diff --git a/src/shared/dissect-image.c b/src/shared/dissect-image.c index de94d3399c..62050daecd 100644 --- a/src/shared/dissect-image.c +++ b/src/shared/dissect-image.c @@ -1681,12 +1681,12 @@ static int verity_can_reuse( return 0; } -static inline void dm_deferred_remove_clean(char *name) { +static inline char* dm_deferred_remove_clean(char *name) { if (!name) - return; + return NULL; (void) sym_crypt_deactivate_by_name(NULL, name, CRYPT_DEACTIVATE_DEFERRED); - free(name); + return mfree(name); } DEFINE_TRIVIAL_CLEANUP_FUNC(char *, dm_deferred_remove_clean); diff --git a/src/shared/install.c b/src/shared/install.c index e70fa45022..77208582b7 100644 --- a/src/shared/install.c +++ b/src/shared/install.c @@ -3346,12 +3346,12 @@ int unit_file_preset_all( return execute_preset(scope, &plus, &minus, &paths, config_path, NULL, mode, !!(flags & UNIT_FILE_FORCE), changes, n_changes); } -static void unit_file_list_free_one(UnitFileList *f) { +static UnitFileList* unit_file_list_free_one(UnitFileList *f) { if (!f) - return; + return NULL; free(f->path); - free(f); + return mfree(f); } Hashmap* unit_file_list_free(Hashmap *h) { diff --git a/src/systemctl/systemctl-show.c b/src/systemctl/systemctl-show.c index 57e708095d..6f3b3566d7 100644 --- a/src/systemctl/systemctl-show.c +++ b/src/systemctl/systemctl-show.c @@ -147,15 +147,14 @@ typedef struct UnitCondition { LIST_FIELDS(struct UnitCondition, conditions); } UnitCondition; -static void unit_condition_free(UnitCondition *c) { +static UnitCondition* unit_condition_free(UnitCondition *c) { if (!c) - return; + return NULL; free(c->name); free(c->param); - free(c); + return mfree(c); } - DEFINE_TRIVIAL_CLEANUP_FUNC(UnitCondition*, unit_condition_free); typedef struct UnitStatusInfo { diff --git a/src/sysv-generator/sysv-generator.c b/src/sysv-generator/sysv-generator.c index 008a8250bb..8c7aef23c3 100644 --- a/src/sysv-generator/sysv-generator.c +++ b/src/sysv-generator/sysv-generator.c @@ -59,9 +59,9 @@ typedef struct SysvStub { bool loaded; } SysvStub; -static void free_sysvstub(SysvStub *s) { +static SysvStub* free_sysvstub(SysvStub *s) { if (!s) - return; + return NULL; free(s->name); free(s->path); @@ -71,9 +71,8 @@ static void free_sysvstub(SysvStub *s) { strv_free(s->after); strv_free(s->wants); strv_free(s->wanted_by); - free(s); + return mfree(s); } - DEFINE_TRIVIAL_CLEANUP_FUNC(SysvStub*, free_sysvstub); static void free_sysvstub_hashmapp(Hashmap **h) { diff --git a/src/timesync/timesyncd-manager.c b/src/timesync/timesyncd-manager.c index 07fee57391..8c2ee4cbec 100644 --- a/src/timesync/timesyncd-manager.c +++ b/src/timesync/timesyncd-manager.c @@ -915,9 +915,9 @@ void manager_flush_server_names(Manager *m, ServerType t) { server_name_free(m->fallback_servers); } -void manager_free(Manager *m) { +Manager* manager_free(Manager *m) { if (!m) - return; + return NULL; manager_disconnect(m); manager_flush_server_names(m, SERVER_SYSTEM); @@ -934,7 +934,7 @@ void manager_free(Manager *m) { sd_bus_flush_close_unref(m->bus); - free(m); + return mfree(m); } static int manager_network_read_link_servers(Manager *m) { diff --git a/src/timesync/timesyncd-manager.h b/src/timesync/timesyncd-manager.h index 37cf406190..af218a5f63 100644 --- a/src/timesync/timesyncd-manager.h +++ b/src/timesync/timesyncd-manager.h @@ -103,7 +103,7 @@ struct Manager { }; int manager_new(Manager **ret); -void manager_free(Manager *m); +Manager* manager_free(Manager *m); DEFINE_TRIVIAL_CLEANUP_FUNC(Manager*, manager_free); diff --git a/src/udev/net/link-config.c b/src/udev/net/link-config.c index 9209f8f4b4..f2ca1bb561 100644 --- a/src/udev/net/link-config.c +++ b/src/udev/net/link-config.c @@ -45,9 +45,9 @@ struct link_config_ctx { usec_t network_dirs_ts_usec; }; -static void link_config_free(link_config *link) { +static link_config* link_config_free(link_config *link) { if (!link) - return; + return NULL; free(link->filename); @@ -62,7 +62,7 @@ static void link_config_free(link_config *link) { free(link->alternative_names_policy); free(link->alias); - free(link); + return mfree(link); } DEFINE_TRIVIAL_CLEANUP_FUNC(link_config*, link_config_free); @@ -77,19 +77,14 @@ static void link_configs_free(link_config_ctx *ctx) { link_config_free(link); } -void link_config_ctx_free(link_config_ctx *ctx) { +link_config_ctx* link_config_ctx_free(link_config_ctx *ctx) { if (!ctx) - return; + return NULL; safe_close(ctx->ethtool_fd); - sd_netlink_unref(ctx->rtnl); - link_configs_free(ctx); - - free(ctx); - - return; + return mfree(ctx); } int link_config_ctx_new(link_config_ctx **ret) { diff --git a/src/udev/net/link-config.h b/src/udev/net/link-config.h index 3f899e9879..71f8983fb7 100644 --- a/src/udev/net/link-config.h +++ b/src/udev/net/link-config.h @@ -69,7 +69,7 @@ struct link_config { }; int link_config_ctx_new(link_config_ctx **ret); -void link_config_ctx_free(link_config_ctx *ctx); +link_config_ctx* link_config_ctx_free(link_config_ctx *ctx); DEFINE_TRIVIAL_CLEANUP_FUNC(link_config_ctx*, link_config_ctx_free); int link_load_one(link_config_ctx *ctx, const char *filename); diff --git a/src/udev/udev-ctrl.c b/src/udev/udev-ctrl.c index c65f8772ea..61090a6e4a 100644 --- a/src/udev/udev-ctrl.c +++ b/src/udev/udev-ctrl.c @@ -164,6 +164,7 @@ static void udev_ctrl_disconnect_and_listen_again(struct udev_ctrl *uctrl) { udev_ctrl_disconnect(uctrl); udev_ctrl_unref(uctrl); (void) sd_event_source_set_enabled(uctrl->event_source, SD_EVENT_ON); + /* We don't return NULL here because uctrl is not freed */ } DEFINE_TRIVIAL_CLEANUP_FUNC(struct udev_ctrl *, udev_ctrl_disconnect_and_listen_again); diff --git a/src/udev/udev-rules.c b/src/udev/udev-rules.c index fb4de17a5c..78184b4136 100644 --- a/src/udev/udev-rules.c +++ b/src/udev/udev-rules.c @@ -264,9 +264,9 @@ static void udev_rule_line_clear_tokens(UdevRuleLine *rule_line) { rule_line->tokens = NULL; } -static void udev_rule_line_free(UdevRuleLine *rule_line) { +static UdevRuleLine* udev_rule_line_free(UdevRuleLine *rule_line) { if (!rule_line) - return; + return NULL; udev_rule_line_clear_tokens(rule_line); @@ -278,7 +278,7 @@ static void udev_rule_line_free(UdevRuleLine *rule_line) { } free(rule_line->line); - free(rule_line); + return mfree(rule_line); } DEFINE_TRIVIAL_CLEANUP_FUNC(UdevRuleLine*, udev_rule_line_free); diff --git a/src/udev/udevd.c b/src/udev/udevd.c index 0134f78b4b..9772246aeb 100644 --- a/src/udev/udevd.c +++ b/src/udev/udevd.c @@ -173,9 +173,9 @@ static void event_free(struct event *event) { free(event); } -static void worker_free(struct worker *worker) { +static struct worker* worker_free(struct worker *worker) { if (!worker) - return; + return NULL; assert(worker->manager); @@ -183,7 +183,7 @@ static void worker_free(struct worker *worker) { sd_device_monitor_unref(worker->monitor); event_free(worker->event); - free(worker); + return mfree(worker); } DEFINE_TRIVIAL_CLEANUP_FUNC(struct worker *, worker_free); @@ -287,9 +287,9 @@ static void manager_clear_for_worker(Manager *manager) { manager->worker_watch[READ_END] = safe_close(manager->worker_watch[READ_END]); } -static void manager_free(Manager *manager) { +static Manager* manager_free(Manager *manager) { if (!manager) - return; + return NULL; udev_builtin_exit(); @@ -306,7 +306,7 @@ static void manager_free(Manager *manager) { safe_close(manager->fd_inotify); safe_close_pair(manager->worker_watch); - free(manager); + return mfree(manager); } DEFINE_TRIVIAL_CLEANUP_FUNC(Manager*, manager_free); From fd421c4adc7406de02e03e2f7ceede2cc6e1d9d4 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Zbigniew=20J=C4=99drzejewski-Szmek?= Date: Tue, 16 Feb 2021 14:18:30 +0100 Subject: [PATCH 09/13] tree-wide: reset the cleaned-up variable in cleanup functions If the cleanup function returns the appropriate type, use that to reset the variable. For other functions (usually the foreign ones which return void), add an explicit value to reset to. This causes a bit of code churn, but I think it might be worth it. In a following patch static destructors will be called from a fuzzer, and this change allows them to be called multiple times. But I think such a change might help with detecting unitialized code reuse too. We hit various bugs like this, and things are more obvious when a pointer has been set to NULL. I was worried whether this change increases text size, but it doesn't seem to: -Dbuildtype=debug: before "tree-wide: return NULL from freeing functions": -rwxrwxr-x 1 zbyszek zbyszek 4117672 Feb 16 14:36 build/libsystemd.so.0.30.0* -rwxrwxr-x 1 zbyszek zbyszek 4494520 Feb 16 15:06 build/systemd* after "tree-wide: return NULL from freeing functions": -rwxrwxr-x 1 zbyszek zbyszek 4117672 Feb 16 14:36 build/libsystemd.so.0.30.0* -rwxrwxr-x 1 zbyszek zbyszek 4494576 Feb 16 15:10 build/systemd* now: -rwxrwxr-x 1 zbyszek zbyszek 4117672 Feb 16 14:36 build/libsystemd.so.0.30.0* -rwxrwxr-x 1 zbyszek zbyszek 4494640 Feb 16 15:15 build/systemd* -Dbuildtype=release: before "tree-wide: return NULL from freeing functions": -rwxrwxr-x 1 zbyszek zbyszek 5252256 Feb 14 14:47 build-rawhide/libsystemd.so.0.30.0* -rwxrwxr-x 1 zbyszek zbyszek 1834184 Feb 16 15:09 build-rawhide/systemd* after "tree-wide: return NULL from freeing functions": -rwxrwxr-x 1 zbyszek zbyszek 5252256 Feb 14 14:47 build-rawhide/libsystemd.so.0.30.0* -rwxrwxr-x 1 zbyszek zbyszek 1834184 Feb 16 15:10 build-rawhide/systemd* now: -rwxrwxr-x 1 zbyszek zbyszek 5252256 Feb 14 14:47 build-rawhide/libsystemd.so.0.30.0* -rwxrwxr-x 1 zbyszek zbyszek 1834184 Feb 16 15:16 build-rawhide/systemd* I would expect that the compiler would be able to elide the setting of a variable if the variable is never used again. And this seems to be the case: in optimized builds there is no change in size whatsoever. And the change in size in unoptimized build is negligible. Something strange is happening with size of libsystemd: it's bigger in optimized builds. Something to figure out, but unrelated to this patch. --- src/basic/capability-util.h | 2 +- src/basic/dlfcn-util.h | 2 +- src/basic/fd-util.h | 4 ++-- src/basic/fileio.c | 2 +- src/basic/gcrypt-util.h | 2 +- src/basic/macro.h | 10 ++++++++++ src/basic/selinux-util.c | 2 +- src/basic/selinux-util.h | 2 +- src/core/apparmor-setup.c | 6 +++--- src/core/socket.c | 2 +- src/home/homed-manager.c | 4 ++-- src/home/homework-luks.c | 8 ++++---- src/home/user-record-sign.c | 2 +- src/import/curl-util.h | 6 +++--- src/journal-remote/microhttpd-util.h | 4 ++-- src/journal/journalctl.c | 4 ++-- src/libsystemd/sd-journal/compress.c | 8 ++++---- src/partition/repart.c | 8 ++++---- src/resolve/resolved-dnstls-gnutls.c | 2 +- src/shared/acl-util.h | 8 ++++---- src/shared/blkid-util.h | 2 +- src/shared/cryptsetup-util.h | 4 ++-- src/shared/firewall-util-iptables.c | 2 +- src/shared/libmount-util.h | 4 ++-- src/shared/module-util.h | 2 +- src/shared/mount-util.h | 2 +- src/shared/openssl-util.h | 8 ++++---- src/shared/pkcs11-util.h | 4 ++-- src/shared/pwquality-util.h | 2 +- src/shared/seccomp-util.h | 2 +- src/udev/udev-ctrl.c | 2 +- 31 files changed, 66 insertions(+), 56 deletions(-) diff --git a/src/basic/capability-util.h b/src/basic/capability-util.h index f5ce290524..d9489d7883 100644 --- a/src/basic/capability-util.h +++ b/src/basic/capability-util.h @@ -25,7 +25,7 @@ int drop_privileges(uid_t uid, gid_t gid, uint64_t keep_capabilities); int drop_capability(cap_value_t cv); -DEFINE_TRIVIAL_CLEANUP_FUNC(cap_t, cap_free); +DEFINE_TRIVIAL_CLEANUP_FUNC_FULL(cap_t, cap_free, NULL); #define _cleanup_cap_free_ _cleanup_(cap_freep) static inline void cap_free_charpp(char **p) { diff --git a/src/basic/dlfcn-util.h b/src/basic/dlfcn-util.h index e0944f3aaf..aa713d328b 100644 --- a/src/basic/dlfcn-util.h +++ b/src/basic/dlfcn-util.h @@ -5,7 +5,7 @@ #include "macro.h" -DEFINE_TRIVIAL_CLEANUP_FUNC(void*, dlclose); +DEFINE_TRIVIAL_CLEANUP_FUNC_FULL(void*, dlclose, NULL); int dlsym_many_and_warn(void *dl, int level, ...); diff --git a/src/basic/fd-util.h b/src/basic/fd-util.h index 2162537b80..f05c2b5a15 100644 --- a/src/basic/fd-util.h +++ b/src/basic/fd-util.h @@ -41,8 +41,8 @@ static inline void fclosep(FILE **f) { safe_fclose(*f); } -DEFINE_TRIVIAL_CLEANUP_FUNC(FILE*, pclose); -DEFINE_TRIVIAL_CLEANUP_FUNC(DIR*, closedir); +DEFINE_TRIVIAL_CLEANUP_FUNC_FULL(FILE*, pclose, NULL); +DEFINE_TRIVIAL_CLEANUP_FUNC_FULL(DIR*, closedir, NULL); #define _cleanup_close_ _cleanup_(closep) #define _cleanup_fclose_ _cleanup_(fclosep) diff --git a/src/basic/fileio.c b/src/basic/fileio.c index f4708bc05f..21532515fb 100644 --- a/src/basic/fileio.c +++ b/src/basic/fileio.c @@ -1146,7 +1146,7 @@ static EndOfLineMarker categorize_eol(char c, ReadLineFlags flags) { return EOL_NONE; } -DEFINE_TRIVIAL_CLEANUP_FUNC(FILE*, funlockfile); +DEFINE_TRIVIAL_CLEANUP_FUNC_FULL(FILE*, funlockfile, NULL); int read_line_full(FILE *f, size_t limit, ReadLineFlags flags, char **ret) { size_t n = 0, allocated = 0, count = 0; diff --git a/src/basic/gcrypt-util.h b/src/basic/gcrypt-util.h index c07b36cdb9..27dcc72028 100644 --- a/src/basic/gcrypt-util.h +++ b/src/basic/gcrypt-util.h @@ -14,7 +14,7 @@ void initialize_libgcrypt(bool secmem); int string_hashsum(const char *s, size_t len, int md_algorithm, char **out); -DEFINE_TRIVIAL_CLEANUP_FUNC(gcry_md_hd_t, gcry_md_close); +DEFINE_TRIVIAL_CLEANUP_FUNC_FULL(gcry_md_hd_t, gcry_md_close, NULL); #endif static inline int string_hashsum_sha224(const char *s, size_t len, char **out) { diff --git a/src/basic/macro.h b/src/basic/macro.h index d4e3d3b8fb..eebd0769dd 100644 --- a/src/basic/macro.h +++ b/src/basic/macro.h @@ -405,10 +405,20 @@ static inline int __coverity_check_and_return__(int condition) { func(p); \ } +/* When func() returns the void value (NULL, -1, …) of the appropriate type */ #define DEFINE_TRIVIAL_CLEANUP_FUNC(type, func) \ static inline void func##p(type *p) { \ if (*p) \ + *p = func(*p); \ + } + +/* When func() doesn't return the appropriate type, set variable to empty afterwards */ +#define DEFINE_TRIVIAL_CLEANUP_FUNC_FULL(type, func, empty) \ + static inline void func##p(type *p) { \ + if (*p != (empty)) { \ func(*p); \ + *p = (empty); \ + } \ } #define _DEFINE_TRIVIAL_REF_FUNC(type, name, scope) \ diff --git a/src/basic/selinux-util.c b/src/basic/selinux-util.c index bd22500b7e..cfc8464c66 100644 --- a/src/basic/selinux-util.c +++ b/src/basic/selinux-util.c @@ -29,7 +29,7 @@ #include "time-util.h" #if HAVE_SELINUX -DEFINE_TRIVIAL_CLEANUP_FUNC(context_t, context_free); +DEFINE_TRIVIAL_CLEANUP_FUNC_FULL(context_t, context_free, NULL); #define _cleanup_context_free_ _cleanup_(context_freep) static int mac_selinux_reload(int seqno); diff --git a/src/basic/selinux-util.h b/src/basic/selinux-util.h index 1236d6efdf..1095bdef0e 100644 --- a/src/basic/selinux-util.h +++ b/src/basic/selinux-util.h @@ -11,7 +11,7 @@ #if HAVE_SELINUX #include -DEFINE_TRIVIAL_CLEANUP_FUNC(char*, freecon); +DEFINE_TRIVIAL_CLEANUP_FUNC_FULL(char*, freecon, NULL); #define _cleanup_freecon_ _cleanup_(freeconp) #endif diff --git a/src/core/apparmor-setup.c b/src/core/apparmor-setup.c index e856f5c183..304a3e6aac 100644 --- a/src/core/apparmor-setup.c +++ b/src/core/apparmor-setup.c @@ -16,17 +16,17 @@ #include "strv.h" #if HAVE_APPARMOR -DEFINE_TRIVIAL_CLEANUP_FUNC(aa_policy_cache *, aa_policy_cache_unref); -DEFINE_TRIVIAL_CLEANUP_FUNC(aa_features *, aa_features_unref); +DEFINE_TRIVIAL_CLEANUP_FUNC_FULL(aa_policy_cache *, aa_policy_cache_unref, NULL); +DEFINE_TRIVIAL_CLEANUP_FUNC_FULL(aa_features *, aa_features_unref, NULL); #endif int mac_apparmor_setup(void) { #if HAVE_APPARMOR - int r; _cleanup_(aa_policy_cache_unrefp) aa_policy_cache *policy_cache = NULL; _cleanup_(aa_features_unrefp) aa_features *features = NULL; const char *current_file; _cleanup_free_ char *current_profile = NULL, *cache_dir_path = NULL; + int r; if (!mac_apparmor_use()) { log_debug("AppArmor either not supported by the kernel or disabled."); diff --git a/src/core/socket.c b/src/core/socket.c index 5631054cce..e9cacfdf3a 100644 --- a/src/core/socket.c +++ b/src/core/socket.c @@ -1611,7 +1611,7 @@ static int socket_address_listen_in_cgroup( return fd; } -DEFINE_TRIVIAL_CLEANUP_FUNC(Socket *, socket_close_fds); +DEFINE_TRIVIAL_CLEANUP_FUNC_FULL(Socket *, socket_close_fds, NULL); static int socket_open_fds(Socket *orig_s) { _cleanup_(socket_close_fdsp) Socket *s = orig_s; diff --git a/src/home/homed-manager.c b/src/home/homed-manager.c index 91cabd5cef..1b13002627 100644 --- a/src/home/homed-manager.c +++ b/src/home/homed-manager.c @@ -1343,7 +1343,7 @@ static int manager_load_key_pair(Manager *m) { return 1; } -DEFINE_TRIVIAL_CLEANUP_FUNC(EVP_PKEY_CTX*, EVP_PKEY_CTX_free); +DEFINE_TRIVIAL_CLEANUP_FUNC_FULL(EVP_PKEY_CTX*, EVP_PKEY_CTX_free, NULL); static int manager_generate_key_pair(Manager *m) { _cleanup_(EVP_PKEY_CTX_freep) EVP_PKEY_CTX *ctx = NULL; @@ -1454,7 +1454,7 @@ int manager_sign_user_record(Manager *m, UserRecord *u, UserRecord **ret, sd_bus } DEFINE_PRIVATE_HASH_OPS_FULL(public_key_hash_ops, char, string_hash_func, string_compare_func, free, EVP_PKEY, EVP_PKEY_free); -DEFINE_TRIVIAL_CLEANUP_FUNC(EVP_PKEY*, EVP_PKEY_free); +DEFINE_TRIVIAL_CLEANUP_FUNC_FULL(EVP_PKEY*, EVP_PKEY_free, NULL); static int manager_load_public_key_one(Manager *m, const char *path) { _cleanup_(EVP_PKEY_freep) EVP_PKEY *pkey = NULL; diff --git a/src/home/homework-luks.c b/src/home/homework-luks.c index c6b34d28d4..1d012c5d02 100644 --- a/src/home/homework-luks.c +++ b/src/home/homework-luks.c @@ -1592,10 +1592,10 @@ static int luks_format( return 0; } -DEFINE_TRIVIAL_CLEANUP_FUNC(struct fdisk_context*, fdisk_unref_context); -DEFINE_TRIVIAL_CLEANUP_FUNC(struct fdisk_partition*, fdisk_unref_partition); -DEFINE_TRIVIAL_CLEANUP_FUNC(struct fdisk_parttype*, fdisk_unref_parttype); -DEFINE_TRIVIAL_CLEANUP_FUNC(struct fdisk_table*, fdisk_unref_table); +DEFINE_TRIVIAL_CLEANUP_FUNC_FULL(struct fdisk_context*, fdisk_unref_context, NULL); +DEFINE_TRIVIAL_CLEANUP_FUNC_FULL(struct fdisk_partition*, fdisk_unref_partition, NULL); +DEFINE_TRIVIAL_CLEANUP_FUNC_FULL(struct fdisk_parttype*, fdisk_unref_parttype, NULL); +DEFINE_TRIVIAL_CLEANUP_FUNC_FULL(struct fdisk_table*, fdisk_unref_table, NULL); static int make_partition_table( int fd, diff --git a/src/home/user-record-sign.c b/src/home/user-record-sign.c index 8cd3a46251..5ac92255c8 100644 --- a/src/home/user-record-sign.c +++ b/src/home/user-record-sign.c @@ -27,7 +27,7 @@ static int user_record_signable_json(UserRecord *ur, char **ret) { return json_variant_format(j, 0, ret); } -DEFINE_TRIVIAL_CLEANUP_FUNC(EVP_MD_CTX*, EVP_MD_CTX_free); +DEFINE_TRIVIAL_CLEANUP_FUNC_FULL(EVP_MD_CTX*, EVP_MD_CTX_free, NULL); int user_record_sign(UserRecord *ur, EVP_PKEY *private_key, UserRecord **ret) { _cleanup_(json_variant_unrefp) JsonVariant *encoded = NULL, *v = NULL; diff --git a/src/import/curl-util.h b/src/import/curl-util.h index 4ab52d73a4..6b4f992fe7 100644 --- a/src/import/curl-util.h +++ b/src/import/curl-util.h @@ -34,6 +34,6 @@ struct curl_slist *curl_slist_new(const char *first, ...) _sentinel_; int curl_header_strdup(const void *contents, size_t sz, const char *field, char **value); int curl_parse_http_time(const char *t, usec_t *ret); -DEFINE_TRIVIAL_CLEANUP_FUNC(CURL*, curl_easy_cleanup); -DEFINE_TRIVIAL_CLEANUP_FUNC(CURLM*, curl_multi_cleanup); -DEFINE_TRIVIAL_CLEANUP_FUNC(struct curl_slist*, curl_slist_free_all); +DEFINE_TRIVIAL_CLEANUP_FUNC_FULL(CURL*, curl_easy_cleanup, NULL); +DEFINE_TRIVIAL_CLEANUP_FUNC_FULL(CURLM*, curl_multi_cleanup, NULL); +DEFINE_TRIVIAL_CLEANUP_FUNC_FULL(struct curl_slist*, curl_slist_free_all, NULL); diff --git a/src/journal-remote/microhttpd-util.h b/src/journal-remote/microhttpd-util.h index 7f90a09c77..a92ba57d0f 100644 --- a/src/journal-remote/microhttpd-util.h +++ b/src/journal-remote/microhttpd-util.h @@ -80,5 +80,5 @@ int check_permissions(struct MHD_Connection *connection, int *code, char **hostn */ int setup_gnutls_logger(char **categories); -DEFINE_TRIVIAL_CLEANUP_FUNC(struct MHD_Daemon*, MHD_stop_daemon); -DEFINE_TRIVIAL_CLEANUP_FUNC(struct MHD_Response*, MHD_destroy_response); +DEFINE_TRIVIAL_CLEANUP_FUNC_FULL(struct MHD_Daemon*, MHD_stop_daemon, NULL); +DEFINE_TRIVIAL_CLEANUP_FUNC_FULL(struct MHD_Response*, MHD_destroy_response, NULL); diff --git a/src/journal/journalctl.c b/src/journal/journalctl.c index a1b7bee2a3..3de3b4e26f 100644 --- a/src/journal/journalctl.c +++ b/src/journal/journalctl.c @@ -166,8 +166,8 @@ typedef struct BootId { } BootId; #if HAVE_PCRE2 -DEFINE_TRIVIAL_CLEANUP_FUNC(pcre2_match_data*, sym_pcre2_match_data_free); -DEFINE_TRIVIAL_CLEANUP_FUNC(pcre2_code*, sym_pcre2_code_free); +DEFINE_TRIVIAL_CLEANUP_FUNC_FULL(pcre2_match_data*, sym_pcre2_match_data_free, NULL); +DEFINE_TRIVIAL_CLEANUP_FUNC_FULL(pcre2_code*, sym_pcre2_code_free, NULL); static int pattern_compile(const char *pattern, unsigned flags, pcre2_code **out) { int errorcode, r; diff --git a/src/libsystemd/sd-journal/compress.c b/src/libsystemd/sd-journal/compress.c index 429dbadb6c..3ea3328c1e 100644 --- a/src/libsystemd/sd-journal/compress.c +++ b/src/libsystemd/sd-journal/compress.c @@ -34,13 +34,13 @@ #include "util.h" #if HAVE_LZ4 -DEFINE_TRIVIAL_CLEANUP_FUNC(LZ4F_compressionContext_t, LZ4F_freeCompressionContext); -DEFINE_TRIVIAL_CLEANUP_FUNC(LZ4F_decompressionContext_t, LZ4F_freeDecompressionContext); +DEFINE_TRIVIAL_CLEANUP_FUNC_FULL(LZ4F_compressionContext_t, LZ4F_freeCompressionContext, NULL); +DEFINE_TRIVIAL_CLEANUP_FUNC_FULL(LZ4F_decompressionContext_t, LZ4F_freeDecompressionContext, NULL); #endif #if HAVE_ZSTD -DEFINE_TRIVIAL_CLEANUP_FUNC(ZSTD_CCtx *, ZSTD_freeCCtx); -DEFINE_TRIVIAL_CLEANUP_FUNC(ZSTD_DCtx *, ZSTD_freeDCtx); +DEFINE_TRIVIAL_CLEANUP_FUNC_FULL(ZSTD_CCtx*, ZSTD_freeCCtx, NULL); +DEFINE_TRIVIAL_CLEANUP_FUNC_FULL(ZSTD_DCtx*, ZSTD_freeDCtx, NULL); static int zstd_ret_to_errno(size_t ret) { switch (ZSTD_getErrorCode(ret)) { diff --git a/src/partition/repart.c b/src/partition/repart.c index 78c31187ea..01e60e2133 100644 --- a/src/partition/repart.c +++ b/src/partition/repart.c @@ -1268,10 +1268,10 @@ static int context_read_definitions( return 0; } -DEFINE_TRIVIAL_CLEANUP_FUNC(struct fdisk_context*, fdisk_unref_context); -DEFINE_TRIVIAL_CLEANUP_FUNC(struct fdisk_partition*, fdisk_unref_partition); -DEFINE_TRIVIAL_CLEANUP_FUNC(struct fdisk_parttype*, fdisk_unref_parttype); -DEFINE_TRIVIAL_CLEANUP_FUNC(struct fdisk_table*, fdisk_unref_table); +DEFINE_TRIVIAL_CLEANUP_FUNC_FULL(struct fdisk_context*, fdisk_unref_context, NULL); +DEFINE_TRIVIAL_CLEANUP_FUNC_FULL(struct fdisk_partition*, fdisk_unref_partition, NULL); +DEFINE_TRIVIAL_CLEANUP_FUNC_FULL(struct fdisk_parttype*, fdisk_unref_parttype, NULL); +DEFINE_TRIVIAL_CLEANUP_FUNC_FULL(struct fdisk_table*, fdisk_unref_table, NULL); static int determine_current_padding( struct fdisk_context *c, diff --git a/src/resolve/resolved-dnstls-gnutls.c b/src/resolve/resolved-dnstls-gnutls.c index d3edd350b3..e7ccba934e 100644 --- a/src/resolve/resolved-dnstls-gnutls.c +++ b/src/resolve/resolved-dnstls-gnutls.c @@ -11,7 +11,7 @@ #include "resolved-manager.h" #define TLS_PROTOCOL_PRIORITY "NORMAL:-VERS-ALL:+VERS-TLS1.3:+VERS-TLS1.2" -DEFINE_TRIVIAL_CLEANUP_FUNC(gnutls_session_t, gnutls_deinit); +DEFINE_TRIVIAL_CLEANUP_FUNC_FULL(gnutls_session_t, gnutls_deinit, NULL); static ssize_t dnstls_stream_writev(gnutls_transport_ptr_t p, const giovec_t *iov, int iovcnt) { int r; diff --git a/src/shared/acl-util.h b/src/shared/acl-util.h index 837e8699ee..03595c6651 100644 --- a/src/shared/acl-util.h +++ b/src/shared/acl-util.h @@ -21,13 +21,13 @@ int fd_add_uid_acl_permission(int fd, uid_t uid, unsigned mask); /* acl_free takes multiple argument types. * Multiple cleanup functions are necessary. */ -DEFINE_TRIVIAL_CLEANUP_FUNC(acl_t, acl_free); +DEFINE_TRIVIAL_CLEANUP_FUNC_FULL(acl_t, acl_free, NULL); #define acl_free_charp acl_free -DEFINE_TRIVIAL_CLEANUP_FUNC(char*, acl_free_charp); +DEFINE_TRIVIAL_CLEANUP_FUNC_FULL(char*, acl_free_charp, NULL); #define acl_free_uid_tp acl_free -DEFINE_TRIVIAL_CLEANUP_FUNC(uid_t*, acl_free_uid_tp); +DEFINE_TRIVIAL_CLEANUP_FUNC_FULL(uid_t*, acl_free_uid_tp, NULL); #define acl_free_gid_tp acl_free -DEFINE_TRIVIAL_CLEANUP_FUNC(gid_t*, acl_free_gid_tp); +DEFINE_TRIVIAL_CLEANUP_FUNC_FULL(gid_t*, acl_free_gid_tp, NULL); #else #define ACL_READ 0x04 diff --git a/src/shared/blkid-util.h b/src/shared/blkid-util.h index 3f38e9b308..aa444990fd 100644 --- a/src/shared/blkid-util.h +++ b/src/shared/blkid-util.h @@ -6,5 +6,5 @@ # include "macro.h" -DEFINE_TRIVIAL_CLEANUP_FUNC(blkid_probe, blkid_free_probe); +DEFINE_TRIVIAL_CLEANUP_FUNC_FULL(blkid_probe, blkid_free_probe, NULL); #endif diff --git a/src/shared/cryptsetup-util.h b/src/shared/cryptsetup-util.h index 454bfa4532..5ebb0ac576 100644 --- a/src/shared/cryptsetup-util.h +++ b/src/shared/cryptsetup-util.h @@ -50,8 +50,8 @@ static inline int sym_crypt_token_max(_unused_ const char *type) { int dlopen_cryptsetup(void); -DEFINE_TRIVIAL_CLEANUP_FUNC(struct crypt_device *, crypt_free); -DEFINE_TRIVIAL_CLEANUP_FUNC(struct crypt_device *, sym_crypt_free); +DEFINE_TRIVIAL_CLEANUP_FUNC_FULL(struct crypt_device *, crypt_free, NULL); +DEFINE_TRIVIAL_CLEANUP_FUNC_FULL(struct crypt_device *, sym_crypt_free, NULL); void cryptsetup_enable_logging(struct crypt_device *cd); diff --git a/src/shared/firewall-util-iptables.c b/src/shared/firewall-util-iptables.c index fec3000e75..982c61d8fb 100644 --- a/src/shared/firewall-util-iptables.c +++ b/src/shared/firewall-util-iptables.c @@ -27,7 +27,7 @@ #include "macro.h" #include "socket-util.h" -DEFINE_TRIVIAL_CLEANUP_FUNC(struct xtc_handle*, iptc_free); +DEFINE_TRIVIAL_CLEANUP_FUNC_FULL(struct xtc_handle*, iptc_free, NULL); static int entry_fill_basics( struct ipt_entry *entry, diff --git a/src/shared/libmount-util.h b/src/shared/libmount-util.h index db9728c332..cf19c56ed1 100644 --- a/src/shared/libmount-util.h +++ b/src/shared/libmount-util.h @@ -8,8 +8,8 @@ #include "macro.h" -DEFINE_TRIVIAL_CLEANUP_FUNC(struct libmnt_table*, mnt_free_table); -DEFINE_TRIVIAL_CLEANUP_FUNC(struct libmnt_iter*, mnt_free_iter); +DEFINE_TRIVIAL_CLEANUP_FUNC_FULL(struct libmnt_table*, mnt_free_table, NULL); +DEFINE_TRIVIAL_CLEANUP_FUNC_FULL(struct libmnt_iter*, mnt_free_iter, NULL); static inline int libmount_parse( const char *path, diff --git a/src/shared/module-util.h b/src/shared/module-util.h index 4db8c5f04e..8ca6a06a07 100644 --- a/src/shared/module-util.h +++ b/src/shared/module-util.h @@ -7,6 +7,6 @@ DEFINE_TRIVIAL_CLEANUP_FUNC(struct kmod_ctx*, kmod_unref); DEFINE_TRIVIAL_CLEANUP_FUNC(struct kmod_module*, kmod_module_unref); -DEFINE_TRIVIAL_CLEANUP_FUNC(struct kmod_list*, kmod_module_unref_list); +DEFINE_TRIVIAL_CLEANUP_FUNC_FULL(struct kmod_list*, kmod_module_unref_list, NULL); int module_load_and_warn(struct kmod_ctx *ctx, const char *module, bool verbose); diff --git a/src/shared/mount-util.h b/src/shared/mount-util.h index 849c37e85b..7183897686 100644 --- a/src/shared/mount-util.h +++ b/src/shared/mount-util.h @@ -45,7 +45,7 @@ int bind_remount_one_with_mountinfo(const char *path, unsigned long new_flags, u int mount_move_root(const char *path); -DEFINE_TRIVIAL_CLEANUP_FUNC(FILE*, endmntent); +DEFINE_TRIVIAL_CLEANUP_FUNC_FULL(FILE*, endmntent, NULL); #define _cleanup_endmntent_ _cleanup_(endmntentp) int mount_verbose_full( diff --git a/src/shared/openssl-util.h b/src/shared/openssl-util.h index b753a690ba..0f527e74c4 100644 --- a/src/shared/openssl-util.h +++ b/src/shared/openssl-util.h @@ -6,10 +6,10 @@ #if HAVE_OPENSSL # include -DEFINE_TRIVIAL_CLEANUP_FUNC(X509*, X509_free); -DEFINE_TRIVIAL_CLEANUP_FUNC(X509_NAME*, X509_NAME_free); -DEFINE_TRIVIAL_CLEANUP_FUNC(EVP_PKEY_CTX*, EVP_PKEY_CTX_free); -DEFINE_TRIVIAL_CLEANUP_FUNC(EVP_CIPHER_CTX*, EVP_CIPHER_CTX_free); +DEFINE_TRIVIAL_CLEANUP_FUNC_FULL(X509*, X509_free, NULL); +DEFINE_TRIVIAL_CLEANUP_FUNC_FULL(X509_NAME*, X509_NAME_free, NULL); +DEFINE_TRIVIAL_CLEANUP_FUNC_FULL(EVP_PKEY_CTX*, EVP_PKEY_CTX_free, NULL); +DEFINE_TRIVIAL_CLEANUP_FUNC_FULL(EVP_CIPHER_CTX*, EVP_CIPHER_CTX_free, NULL); int rsa_encrypt_bytes(EVP_PKEY *pkey, const void *decrypted_key, size_t decrypted_key_size, void **ret_encrypt_key, size_t *ret_encrypt_key_size); diff --git a/src/shared/pkcs11-util.h b/src/shared/pkcs11-util.h index 3fa2505deb..f32ab30429 100644 --- a/src/shared/pkcs11-util.h +++ b/src/shared/pkcs11-util.h @@ -21,8 +21,8 @@ P11KitUri *uri_from_module_info(const CK_INFO *info); P11KitUri *uri_from_slot_info(const CK_SLOT_INFO *slot_info); P11KitUri *uri_from_token_info(const CK_TOKEN_INFO *token_info); -DEFINE_TRIVIAL_CLEANUP_FUNC(P11KitUri*, p11_kit_uri_free); -DEFINE_TRIVIAL_CLEANUP_FUNC(CK_FUNCTION_LIST**, p11_kit_modules_finalize_and_release); +DEFINE_TRIVIAL_CLEANUP_FUNC_FULL(P11KitUri*, p11_kit_uri_free, NULL); +DEFINE_TRIVIAL_CLEANUP_FUNC_FULL(CK_FUNCTION_LIST**, p11_kit_modules_finalize_and_release, NULL); CK_RV pkcs11_get_slot_list_malloc(CK_FUNCTION_LIST *m, CK_SLOT_ID **ret_slotids, CK_ULONG *ret_n_slotids); diff --git a/src/shared/pwquality-util.h b/src/shared/pwquality-util.h index de288bb017..877f439d11 100644 --- a/src/shared/pwquality-util.h +++ b/src/shared/pwquality-util.h @@ -19,7 +19,7 @@ extern const char* (*sym_pwquality_strerror)(char *buf, size_t len, int errcode, int dlopen_pwquality(void); -DEFINE_TRIVIAL_CLEANUP_FUNC(pwquality_settings_t*, sym_pwquality_free_settings); +DEFINE_TRIVIAL_CLEANUP_FUNC_FULL(pwquality_settings_t*, sym_pwquality_free_settings, NULL); void pwq_maybe_disable_dictionary(pwquality_settings_t *pwq); int pwq_allocate_context(pwquality_settings_t **ret); diff --git a/src/shared/seccomp-util.h b/src/shared/seccomp-util.h index ee6f7b878b..b3d25c9f3f 100644 --- a/src/shared/seccomp-util.h +++ b/src/shared/seccomp-util.h @@ -121,7 +121,7 @@ extern uint32_t seccomp_local_archs[]; #define ERRNO_IS_SECCOMP_FATAL(r) \ IN_SET(abs(r), EPERM, EACCES, ENOMEM, EFAULT) -DEFINE_TRIVIAL_CLEANUP_FUNC(scmp_filter_ctx, seccomp_release); +DEFINE_TRIVIAL_CLEANUP_FUNC_FULL(scmp_filter_ctx, seccomp_release, NULL); int parse_syscall_archs(char **l, Set **ret_archs); diff --git a/src/udev/udev-ctrl.c b/src/udev/udev-ctrl.c index 61090a6e4a..3d56354719 100644 --- a/src/udev/udev-ctrl.c +++ b/src/udev/udev-ctrl.c @@ -167,7 +167,7 @@ static void udev_ctrl_disconnect_and_listen_again(struct udev_ctrl *uctrl) { /* We don't return NULL here because uctrl is not freed */ } -DEFINE_TRIVIAL_CLEANUP_FUNC(struct udev_ctrl *, udev_ctrl_disconnect_and_listen_again); +DEFINE_TRIVIAL_CLEANUP_FUNC_FULL(struct udev_ctrl*, udev_ctrl_disconnect_and_listen_again, NULL); static int udev_ctrl_connection_event_handler(sd_event_source *s, int fd, uint32_t revents, void *userdata) { _cleanup_(udev_ctrl_disconnect_and_listen_againp) struct udev_ctrl *uctrl = NULL; From 36556f6e514095d220d1cdb37697a07f4b6061d3 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Zbigniew=20J=C4=99drzejewski-Szmek?= Date: Tue, 16 Feb 2021 14:19:36 +0100 Subject: [PATCH 10/13] systemctl: use free_and_replace on global variable In normal usage we cannot set it multiple times, but from a fuzzer we may. Doing it this way is nicer anyway. --- src/systemctl/systemctl-compat-shutdown.c | 5 +++-- src/systemctl/systemctl.c | 4 ++-- 2 files changed, 5 insertions(+), 4 deletions(-) diff --git a/src/systemctl/systemctl-compat-shutdown.c b/src/systemctl/systemctl-compat-shutdown.c index d876c79d33..11154f5f8b 100644 --- a/src/systemctl/systemctl-compat-shutdown.c +++ b/src/systemctl/systemctl-compat-shutdown.c @@ -132,9 +132,10 @@ int shutdown_parse_argv(int argc, char *argv[]) { wall = argv + optind + 1; if (wall) { - arg_wall = strv_copy(wall); - if (!arg_wall) + char **copy = strv_copy(wall); + if (!copy) return log_oom(); + strv_free_and_replace(arg_wall, copy); } optind = argc; diff --git a/src/systemctl/systemctl.c b/src/systemctl/systemctl.c index b82e541d7a..b27da5dad9 100644 --- a/src/systemctl/systemctl.c +++ b/src/systemctl/systemctl.c @@ -1159,8 +1159,8 @@ static int run(int argc, char *argv[]) { case ACTION_SUSPEND_THEN_HIBERNATE: case ACTION_EMERGENCY: case ACTION_DEFAULT: - /* systemctl verbs with no equivalent in the legacy commands. These cannot appear in - * arg_action. Fall through. */ + /* systemctl verbs with no equivalent in the legacy commands. These cannot appear in + * arg_action. Fall through. */ case _ACTION_INVALID: default: From 425ac7a253321b8e8d1b0a0f7a173892a3abc385 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Zbigniew=20J=C4=99drzejewski-Szmek?= Date: Tue, 16 Feb 2021 14:27:26 +0100 Subject: [PATCH 11/13] fuzz-systemctl-parse-argv: call static destuctors With all the preparatory work in previous PRs, we can now call static destructors repeatedly without issue. We need to do it here so that global variables allocated during parsing are properly freed. --- src/systemctl/fuzz-systemctl-parse-argv.c | 9 +++++++++ .../missing-strv-free.input | Bin 0 -> 99 bytes 2 files changed, 9 insertions(+) create mode 100644 test/fuzz/fuzz-systemctl-parse-argv/missing-strv-free.input diff --git a/src/systemctl/fuzz-systemctl-parse-argv.c b/src/systemctl/fuzz-systemctl-parse-argv.c index cf66fa4bb7..31269863c1 100644 --- a/src/systemctl/fuzz-systemctl-parse-argv.c +++ b/src/systemctl/fuzz-systemctl-parse-argv.c @@ -6,6 +6,8 @@ #include "env-util.h" #include "fd-util.h" #include "fuzz.h" +#include "selinux-util.h" +#include "static-destruct.h" #include "stdio-util.h" #include "strv.h" #include "systemctl.h" @@ -57,5 +59,12 @@ int LLVMFuzzerTestOneInput(const uint8_t *data, size_t size) { release_busses(); /* We open the bus for communication with logind. * It needs to be closed to avoid apparent leaks. */ + + mac_selinux_finish(); + + /* Call static destructors to do global state cleanup. We do it here, and not in fuzz-main.c so that + * any global state is destoyed between fuzzer runs. */ + static_destruct(); + return 0; } diff --git a/test/fuzz/fuzz-systemctl-parse-argv/missing-strv-free.input b/test/fuzz/fuzz-systemctl-parse-argv/missing-strv-free.input new file mode 100644 index 0000000000000000000000000000000000000000..5600f4fd0080a6f7e6d98834b7a58775f1c57888 GIT binary patch literal 99 zcma*bK?;B{3ona|-(pFvDhrrDm`> O6=E*|Yt6nCf}+1zNgCJy literal 0 HcmV?d00001 From cfb1a0e555051190d8fe57ddc795aca49dcda210 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Zbigniew=20J=C4=99drzejewski-Szmek?= Date: Tue, 16 Feb 2021 17:05:42 +0100 Subject: [PATCH 12/13] Rename strbuf_cleanup to strbuf_free It frees the whole object. --- src/basic/strbuf.c | 2 +- src/basic/strbuf.h | 4 ++-- src/libsystemd/sd-hwdb/hwdb-util.c | 2 +- src/libsystemd/sd-journal/catalog.c | 2 +- src/test/test-strbuf.c | 2 +- 5 files changed, 6 insertions(+), 6 deletions(-) diff --git a/src/basic/strbuf.c b/src/basic/strbuf.c index 535ac58bed..212b561fa6 100644 --- a/src/basic/strbuf.c +++ b/src/basic/strbuf.c @@ -65,7 +65,7 @@ void strbuf_complete(struct strbuf *str) { } /* clean up everything */ -struct strbuf* strbuf_cleanup(struct strbuf *str) { +struct strbuf* strbuf_free(struct strbuf *str) { if (!str) return NULL; diff --git a/src/basic/strbuf.h b/src/basic/strbuf.h index 9a21438d64..6187c08683 100644 --- a/src/basic/strbuf.h +++ b/src/basic/strbuf.h @@ -35,5 +35,5 @@ struct strbuf_child_entry { struct strbuf* strbuf_new(void); ssize_t strbuf_add_string(struct strbuf *str, const char *s, size_t len); void strbuf_complete(struct strbuf *str); -struct strbuf* strbuf_cleanup(struct strbuf *str); -DEFINE_TRIVIAL_CLEANUP_FUNC(struct strbuf*, strbuf_cleanup); +struct strbuf* strbuf_free(struct strbuf *str); +DEFINE_TRIVIAL_CLEANUP_FUNC(struct strbuf*, strbuf_free); diff --git a/src/libsystemd/sd-hwdb/hwdb-util.c b/src/libsystemd/sd-hwdb/hwdb-util.c index 34be0e7fcc..fd45ff0f54 100644 --- a/src/libsystemd/sd-hwdb/hwdb-util.c +++ b/src/libsystemd/sd-hwdb/hwdb-util.c @@ -120,7 +120,7 @@ static struct trie* trie_free(struct trie *trie) { return NULL; trie_node_cleanup(trie->root); - strbuf_cleanup(trie->strings); + strbuf_free(trie->strings); return mfree(trie); } diff --git a/src/libsystemd/sd-journal/catalog.c b/src/libsystemd/sd-journal/catalog.c index 0f6ad8a29e..aea3241d18 100644 --- a/src/libsystemd/sd-journal/catalog.c +++ b/src/libsystemd/sd-journal/catalog.c @@ -444,7 +444,7 @@ error: int catalog_update(const char* database, const char* root, const char* const* dirs) { _cleanup_strv_free_ char **files = NULL; char **f; - _cleanup_(strbuf_cleanupp) struct strbuf *sb = NULL; + _cleanup_(strbuf_freep) struct strbuf *sb = NULL; _cleanup_ordered_hashmap_free_free_free_ OrderedHashmap *h = NULL; _cleanup_free_ CatalogItem *items = NULL; ssize_t offset; diff --git a/src/test/test-strbuf.c b/src/test/test-strbuf.c index 867be19c86..0cdef3fb72 100644 --- a/src/test/test-strbuf.c +++ b/src/test/test-strbuf.c @@ -12,7 +12,7 @@ static ssize_t add_string(struct strbuf *sb, const char *s) { } static void test_strbuf(void) { - _cleanup_(strbuf_cleanupp) struct strbuf *sb; + _cleanup_(strbuf_freep) struct strbuf *sb; _cleanup_strv_free_ char **l; ssize_t a, b, c, d, e, f, g, h; From 15567b3a73285ac9d450fb2a967784e28afb6f5e Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Zbigniew=20J=C4=99drzejewski-Szmek?= Date: Tue, 16 Feb 2021 17:09:00 +0100 Subject: [PATCH 13/13] Rename unit_times_free to unit_times_free_array It frees the whole array and the type is UnitTimes not UnitTime. --- src/analyze/analyze.c | 12 ++++++------ 1 file changed, 6 insertions(+), 6 deletions(-) diff --git a/src/analyze/analyze.c b/src/analyze/analyze.c index 7271adaf5b..3bd0cd5b98 100644 --- a/src/analyze/analyze.c +++ b/src/analyze/analyze.c @@ -213,12 +213,12 @@ static int compare_unit_start(const UnitTimes *a, const UnitTimes *b) { return CMP(a->activating, b->activating); } -static UnitTimes* unit_times_free(UnitTimes *t) { +static UnitTimes* unit_times_free_array(UnitTimes *t) { for (UnitTimes *p = t; p && p->has_data; p++) free(p->name); return mfree(t); } -DEFINE_TRIVIAL_CLEANUP_FUNC(UnitTimes *, unit_times_free); +DEFINE_TRIVIAL_CLEANUP_FUNC(UnitTimes*, unit_times_free_array); static void subtract_timestamp(usec_t *a, usec_t b) { assert(a); @@ -339,7 +339,7 @@ static int acquire_time_data(sd_bus *bus, UnitTimes **out) { }; _cleanup_(sd_bus_message_unrefp) sd_bus_message *reply = NULL; _cleanup_(sd_bus_error_free) sd_bus_error error = SD_BUS_ERROR_NULL; - _cleanup_(unit_times_freep) UnitTimes *unit_times = NULL; + _cleanup_(unit_times_free_arrayp) UnitTimes *unit_times = NULL; BootTimes *boot_times = NULL; size_t allocated = 0, c = 0; UnitInfo u; @@ -614,7 +614,7 @@ static int plot_unit_times(UnitTimes *u, double width, int y) { static int analyze_plot(int argc, char *argv[], void *userdata) { _cleanup_(free_host_infop) HostInfo *host = NULL; _cleanup_(sd_bus_flush_close_unrefp) sd_bus *bus = NULL; - _cleanup_(unit_times_freep) UnitTimes *times = NULL; + _cleanup_(unit_times_free_arrayp) UnitTimes *times = NULL; _cleanup_free_ char *pretty_times = NULL; bool use_full_bus = arg_scope == UNIT_FILE_SYSTEM; BootTimes *boot; @@ -1015,7 +1015,7 @@ static int list_dependencies(sd_bus *bus, const char *name) { static int analyze_critical_chain(int argc, char *argv[], void *userdata) { _cleanup_(sd_bus_flush_close_unrefp) sd_bus *bus = NULL; - _cleanup_(unit_times_freep) UnitTimes *times = NULL; + _cleanup_(unit_times_free_arrayp) UnitTimes *times = NULL; Hashmap *h; int n, r; @@ -1056,7 +1056,7 @@ static int analyze_critical_chain(int argc, char *argv[], void *userdata) { static int analyze_blame(int argc, char *argv[], void *userdata) { _cleanup_(sd_bus_flush_close_unrefp) sd_bus *bus = NULL; - _cleanup_(unit_times_freep) UnitTimes *times = NULL; + _cleanup_(unit_times_free_arrayp) UnitTimes *times = NULL; _cleanup_(table_unrefp) Table *table = NULL; TableCell *cell; int n, r;