From 229b008635a09839cbd5930fe6431397f144b3cb Mon Sep 17 00:00:00 2001 From: David Tardon Date: Wed, 30 Nov 2022 15:18:15 +0100 Subject: [PATCH 1/7] mountpoint-util: reduce variable scope --- src/basic/mountpoint-util.c | 4 +--- 1 file changed, 1 insertion(+), 3 deletions(-) diff --git a/src/basic/mountpoint-util.c b/src/basic/mountpoint-util.c index dc682688a7..c9253b03f2 100644 --- a/src/basic/mountpoint-util.c +++ b/src/basic/mountpoint-util.c @@ -37,7 +37,6 @@ int name_to_handle_at_loop( int *ret_mnt_id, int flags) { - _cleanup_free_ struct file_handle *h = NULL; size_t n = ORIGINAL_MAX_HANDLE_SZ; assert((flags & ~(AT_SYMLINK_FOLLOW|AT_EMPTY_PATH)) == 0); @@ -50,6 +49,7 @@ int name_to_handle_at_loop( * as NULL if there's no interest in either. */ for (;;) { + _cleanup_free_ struct file_handle *h = NULL; int mnt_id = -1; h = malloc0(offsetof(struct file_handle, f_handle) + n); @@ -91,8 +91,6 @@ int name_to_handle_at_loop( n = h->handle_bytes; if (offsetof(struct file_handle, f_handle) + n < n) /* check for addition overflow */ return -EOVERFLOW; - - h = mfree(h); } } From 1dca43db12142441f3f653c72f51c15362ebfb79 Mon Sep 17 00:00:00 2001 From: David Tardon Date: Wed, 30 Nov 2022 15:44:25 +0100 Subject: [PATCH 2/7] ratelimit: drop use of goto --- src/basic/ratelimit.c | 15 +++++---------- 1 file changed, 5 insertions(+), 10 deletions(-) diff --git a/src/basic/ratelimit.c b/src/basic/ratelimit.c index c16c8f7103..f90a63b1a9 100644 --- a/src/basic/ratelimit.c +++ b/src/basic/ratelimit.c @@ -10,6 +10,7 @@ bool ratelimit_below(RateLimit *r) { usec_t ts; + bool good = false; assert(r); @@ -24,18 +25,12 @@ bool ratelimit_below(RateLimit *r) { /* Reset counter */ r->num = 0; - goto good; - } - - if (r->num < r->burst) - goto good; + good = true; + } else if (r->num < r->burst) + good = true; r->num++; - return false; - -good: - r->num++; - return true; + return good; } unsigned ratelimit_num_dropped(RateLimit *r) { From 6323bd1094d495dc939b9095f4650db963cccba5 Mon Sep 17 00:00:00 2001 From: David Tardon Date: Wed, 30 Nov 2022 16:02:11 +0100 Subject: [PATCH 3/7] busctl-introspect: use _cleanup_ --- src/busctl/busctl-introspect.c | 34 +++++++++++----------------------- 1 file changed, 11 insertions(+), 23 deletions(-) diff --git a/src/busctl/busctl-introspect.c b/src/busctl/busctl-introspect.c index a248205f3f..3da4a1390b 100644 --- a/src/busctl/busctl-introspect.c +++ b/src/busctl/busctl-introspect.c @@ -676,7 +676,7 @@ static int parse_xml_node(Context *context, const char *prefix, unsigned n_depth } int parse_xml_introspect(const char *prefix, const char *xml, const XMLIntrospectOps *ops, void *userdata) { - Context context = { + _cleanup_(context_reset_interface) Context context = { .ops = ops, .userdata = userdata, .current = xml, @@ -692,36 +692,24 @@ int parse_xml_introspect(const char *prefix, const char *xml, const XMLIntrospec _cleanup_free_ char *name = NULL; r = xml_tokenize(&context.current, &name, &context.xml_state, NULL); - if (r < 0) { - log_error("XML parse error"); - goto finish; - } + if (r < 0) + return log_error_errno(r, "XML parse error"); - if (r == XML_END) { - r = 0; + if (r == XML_END) break; - } if (r == XML_TAG_OPEN) { if (streq(name, "node")) { r = parse_xml_node(&context, prefix, 0); if (r < 0) - goto finish; - } else { - log_error("Unexpected tag '%s' in introspection data.", name); - r = -EBADMSG; - goto finish; - } - } else if (r != XML_TEXT || !in_charset(name, WHITESPACE)) { - log_error("Unexpected token."); - r = -EBADMSG; - goto finish; - } + return r; + } else + return log_error_errno(SYNTHETIC_ERRNO(EBADMSG), + "Unexpected tag '%s' in introspection data.", name); + } else if (r != XML_TEXT || !in_charset(name, WHITESPACE)) + return log_error_errno(SYNTHETIC_ERRNO(EBADMSG), "Unexpected token."); } -finish: - context_reset_interface(&context); - - return r; + return 0; } From d2b6485385566e84d7e13d272aeeb42b5afc3e34 Mon Sep 17 00:00:00 2001 From: David Tardon Date: Wed, 30 Nov 2022 16:18:06 +0100 Subject: [PATCH 4/7] localed-util: use _cleanup_ harder --- src/locale/localed-util.c | 16 ++++------------ 1 file changed, 4 insertions(+), 12 deletions(-) diff --git a/src/locale/localed-util.c b/src/locale/localed-util.c index 880d0d209c..8fb65b9699 100644 --- a/src/locale/localed-util.c +++ b/src/locale/localed-util.c @@ -254,7 +254,7 @@ int vconsole_write_data(Context *c) { int x11_write_data(Context *c) { _cleanup_fclose_ FILE *f = NULL; - _cleanup_free_ char *temp_path = NULL; + _cleanup_(unlink_and_freep) char *temp_path = NULL; struct stat st; int r; @@ -300,23 +300,15 @@ int x11_write_data(Context *c) { r = fflush_sync_and_check(f); if (r < 0) - goto fail; + return r; - if (rename(temp_path, "/etc/X11/xorg.conf.d/00-keyboard.conf") < 0) { - r = -errno; - goto fail; - } + if (rename(temp_path, "/etc/X11/xorg.conf.d/00-keyboard.conf") < 0) + return -errno; if (stat("/etc/X11/xorg.conf.d/00-keyboard.conf", &st) >= 0) c->x11_mtime = timespec_load(&st.st_mtim); return 0; - -fail: - if (temp_path) - (void) unlink(temp_path); - - return r; } static int read_next_mapping(const char* filename, From 38f514409ac2f98b04a15c6e47a659a43618bd3b Mon Sep 17 00:00:00 2001 From: David Tardon Date: Wed, 30 Nov 2022 16:42:08 +0100 Subject: [PATCH 5/7] machine: use _cleanup_ in machine_new --- src/machine/machine.c | 17 +++++++---------- 1 file changed, 7 insertions(+), 10 deletions(-) diff --git a/src/machine/machine.c b/src/machine/machine.c index de7b20ff2d..ca43c977b0 100644 --- a/src/machine/machine.c +++ b/src/machine/machine.c @@ -33,8 +33,10 @@ #include "unit-name.h" #include "user-util.h" +DEFINE_TRIVIAL_CLEANUP_FUNC(Machine*, machine_free); + Machine* machine_new(Manager *manager, MachineClass class, const char *name) { - Machine *m; + _cleanup_(machine_freep) Machine *m = NULL; assert(manager); assert(class < _MACHINE_CLASS_MAX); @@ -50,27 +52,22 @@ Machine* machine_new(Manager *manager, MachineClass class, const char *name) { m->name = strdup(name); if (!m->name) - goto fail; + return NULL; if (class != MACHINE_HOST) { m->state_file = path_join("/run/systemd/machines", m->name); if (!m->state_file) - goto fail; + return NULL; } m->class = class; if (hashmap_put(manager->machines, m->name, m) < 0) - goto fail; + return NULL; m->manager = manager; - return m; - -fail: - free(m->state_file); - free(m->name); - return mfree(m); + return TAKE_PTR(m); } Machine* machine_free(Machine *m) { From 359e8d76e8addf1d88aa8f8b07a9b09817efbdb6 Mon Sep 17 00:00:00 2001 From: David Tardon Date: Wed, 30 Nov 2022 16:46:05 +0100 Subject: [PATCH 6/7] machine: propagate error from machine_new --- src/machine/machine.c | 18 +++++++++++------- src/machine/machine.h | 2 +- src/machine/machined-dbus.c | 7 ++++--- src/machine/machined.c | 6 +++--- 4 files changed, 19 insertions(+), 14 deletions(-) diff --git a/src/machine/machine.c b/src/machine/machine.c index ca43c977b0..c08a645814 100644 --- a/src/machine/machine.c +++ b/src/machine/machine.c @@ -35,12 +35,14 @@ DEFINE_TRIVIAL_CLEANUP_FUNC(Machine*, machine_free); -Machine* machine_new(Manager *manager, MachineClass class, const char *name) { +int machine_new(Manager *manager, MachineClass class, const char *name, Machine **ret) { _cleanup_(machine_freep) Machine *m = NULL; + int r; assert(manager); assert(class < _MACHINE_CLASS_MAX); assert(name); + assert(ret); /* Passing class == _MACHINE_CLASS_INVALID here is fine. It * means as much as "we don't know yet", and that we'll figure @@ -48,26 +50,28 @@ Machine* machine_new(Manager *manager, MachineClass class, const char *name) { m = new0(Machine, 1); if (!m) - return NULL; + return -ENOMEM; m->name = strdup(name); if (!m->name) - return NULL; + return -ENOMEM; if (class != MACHINE_HOST) { m->state_file = path_join("/run/systemd/machines", m->name); if (!m->state_file) - return NULL; + return -ENOMEM; } m->class = class; - if (hashmap_put(manager->machines, m->name, m) < 0) - return NULL; + r = hashmap_put(manager->machines, m->name, m); + if (r < 0) + return r; m->manager = manager; - return TAKE_PTR(m); + *ret = TAKE_PTR(m); + return 0; } Machine* machine_free(Machine *m) { diff --git a/src/machine/machine.h b/src/machine/machine.h index 5e0e529567..54ebcb3b26 100644 --- a/src/machine/machine.h +++ b/src/machine/machine.h @@ -66,7 +66,7 @@ struct Machine { LIST_FIELDS(Machine, gc_queue); }; -Machine* machine_new(Manager *manager, MachineClass class, const char *name); +int machine_new(Manager *manager, MachineClass class, const char *name, Machine **ret); Machine* machine_free(Machine *m); bool machine_may_gc(Machine *m, bool drop_not_started); void machine_add_to_gc_queue(Machine *m); diff --git a/src/machine/machined-dbus.c b/src/machine/machined-dbus.c index 56dd22d757..3da639279d 100644 --- a/src/machine/machined-dbus.c +++ b/src/machine/machined-dbus.c @@ -1492,15 +1492,16 @@ int manager_get_machine_by_pid(Manager *m, pid_t pid, Machine **machine) { int manager_add_machine(Manager *m, const char *name, Machine **_machine) { Machine *machine; + int r; assert(m); assert(name); machine = hashmap_get(m->machines, name); if (!machine) { - machine = machine_new(m, _MACHINE_CLASS_INVALID, name); - if (!machine) - return -ENOMEM; + r = machine_new(m, _MACHINE_CLASS_INVALID, name, &machine); + if (r < 0) + return r; } if (_machine) diff --git a/src/machine/machined.c b/src/machine/machined.c index 9ecba8720f..b4ff97ab70 100644 --- a/src/machine/machined.c +++ b/src/machine/machined.c @@ -117,9 +117,9 @@ static int manager_add_host_machine(Manager *m) { if (!unit) return log_oom(); - t = machine_new(m, MACHINE_HOST, ".host"); - if (!t) - return log_oom(); + r = machine_new(m, MACHINE_HOST, ".host", &t); + if (r < 0) + return log_error_errno(r, "Failed to create machine: %m"); t->leader = 1; t->id = mid; From 246caacbb41c311b95d8f86da160260a35dace2f Mon Sep 17 00:00:00 2001 From: David Tardon Date: Wed, 30 Nov 2022 16:56:40 +0100 Subject: [PATCH 7/7] btrfs-util: shorten a bit --- src/shared/btrfs-util.c | 5 +---- 1 file changed, 1 insertion(+), 4 deletions(-) diff --git a/src/shared/btrfs-util.c b/src/shared/btrfs-util.c index ba02de17f8..b1aaf746cf 100644 --- a/src/shared/btrfs-util.c +++ b/src/shared/btrfs-util.c @@ -511,10 +511,7 @@ int btrfs_subvol_get_info_fd(int fd, uint64_t subvol_id, BtrfsSubvolInfo *ret) { } finish: - if (!found) - return -ENODATA; - - return 0; + return found ? 0 : -ENODATA; } int btrfs_qgroup_get_quota_fd(int fd, uint64_t qgroupid, BtrfsQuotaInfo *ret) {