From 8b98cfb7dd090b1f5c927012eff3471938f14ac4 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Zbigniew=20J=C4=99drzejewski-Szmek?= Date: Tue, 7 Jun 2022 15:15:17 +0200 Subject: [PATCH 1/8] various: indentation --- src/core/dynamic-user.c | 17 +++++++++-------- src/shared/libfido2-util.c | 5 ++--- 2 files changed, 11 insertions(+), 11 deletions(-) diff --git a/src/core/dynamic-user.c b/src/core/dynamic-user.c index 04d8e7c5e3..18b36e74bf 100644 --- a/src/core/dynamic-user.c +++ b/src/core/dynamic-user.c @@ -328,8 +328,8 @@ static int dynamic_user_pop(DynamicUser *d, uid_t *ret_uid, int *ret_lock_fd) { assert(ret_uid); assert(ret_lock_fd); - /* Read the UID and lock fd that is stored in the storage AF_UNIX socket. This should be called with the lock - * on the socket taken. */ + /* Read the UID and lock fd that is stored in the storage AF_UNIX socket. This should be called with + * the lock on the socket taken. */ k = receive_one_fd_iov(d->storage_socket[0], &iov, 1, MSG_DONTWAIT, &lock_fd); if (k < 0) @@ -531,7 +531,8 @@ int dynamic_user_current(DynamicUser *d, uid_t *ret) { assert(d); - /* Get the currently assigned UID for the user, if there's any. This simply pops the data from the storage socket, and pushes it back in right-away. */ + /* Get the currently assigned UID for the user, if there's any. This simply pops the data from the + * storage socket, and pushes it back in right-away. */ r = lockfp(d->storage_socket[0], &storage_socket0_lock); if (r < 0) @@ -555,9 +556,9 @@ static DynamicUser* dynamic_user_unref(DynamicUser *d) { if (!d) return NULL; - /* Note that this doesn't actually release any resources itself. If a dynamic user should be fully destroyed - * and its UID released, use dynamic_user_destroy() instead. NB: the dynamic user table may contain entries - * with no references, which is commonly the case right before a daemon reload. */ + /* Note that this doesn't actually release any resources itself. If a dynamic user should be fully + * destroyed and its UID released, use dynamic_user_destroy() instead. NB: the dynamic user table may + * contain entries with no references, which is commonly the case right before a daemon reload. */ assert(d->n_ref > 0); d->n_ref--; @@ -571,8 +572,8 @@ static int dynamic_user_close(DynamicUser *d) { uid_t uid; int r; - /* Release the user ID, by releasing the lock on it, and emptying the storage socket. After this the user is - * unrealized again, much like it was after it the DynamicUser object was first allocated. */ + /* Release the user ID, by releasing the lock on it, and emptying the storage socket. After this the + * user is unrealized again, much like it was after it the DynamicUser object was first allocated. */ r = lockfp(d->storage_socket[0], &storage_socket0_lock); if (r < 0) diff --git a/src/shared/libfido2-util.c b/src/shared/libfido2-util.c index 796dc4079e..525849ee19 100644 --- a/src/shared/libfido2-util.c +++ b/src/shared/libfido2-util.c @@ -133,9 +133,8 @@ static int verify_features( assert(path); if (!sym_fido_dev_is_fido2(d)) - return log_full_errno(log_level, - SYNTHETIC_ERRNO(ENODEV), - "Specified device %s is not a FIDO2 device.", path); + return log_full_errno(log_level, SYNTHETIC_ERRNO(ENODEV), + "Specified device %s is not a FIDO2 device.", path); di = sym_fido_cbor_info_new(); if (!di) From d10cee5e9029a14f75eb8d1f63bdf2a97adee2a5 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Zbigniew=20J=C4=99drzejewski-Szmek?= Date: Tue, 7 Jun 2022 15:25:43 +0200 Subject: [PATCH 2/8] test-fstab-util: one more "NULL in printf %s" --- src/test/test-fstab-util.c | 6 +++--- 1 file changed, 3 insertions(+), 3 deletions(-) diff --git a/src/test/test-fstab-util.c b/src/test/test-fstab-util.c index 2ce4e18643..89365b0bc5 100644 --- a/src/test/test-fstab-util.c +++ b/src/test/test-fstab-util.c @@ -37,7 +37,7 @@ static void do_fstab_filter_options(const char *opts, r = fstab_filter_options(opts, remove, &name, &value, NULL, &filtered); log_info("1: \"%s\" → %d, \"%s\", \"%s\", \"%s\", expected %d, \"%s\", \"%s\", \"%s\"", opts, r, strnull(name), value, filtered, - r_expected, name_expected, value_expected, filtered_expected ?: opts); + r_expected, strnull(name_expected), strnull(value_expected), filtered_expected ?: opts); assert_se(r == r_expected); assert_se(streq_ptr(name, name_expected)); assert_se(streq_ptr(value, value_expected)); @@ -49,7 +49,7 @@ static void do_fstab_filter_options(const char *opts, assert_se(joined = strv_join(values, ":")); log_info("2: \"%s\" → %d, \"%s\", \"%s\", expected %d, \"%s\", \"%s\"", opts, r, strnull(name), joined, - r_values_expected, name_expected, values_expected); + r_values_expected, strnull(name_expected), strnull(values_expected)); assert_se(r == r_values_expected); assert_se(streq_ptr(name, r_values_expected > 0 ? name_expected : NULL)); assert_se(streq_ptr(joined, values_expected)); @@ -58,7 +58,7 @@ static void do_fstab_filter_options(const char *opts, r = fstab_filter_options(opts, remove, &name, NULL, NULL, NULL); log_info("3: \"%s\" → %d, \"%s\", expected %d, \"%s\"\n-", opts, r, strnull(name), - r_expected, name_expected); + r_expected, strnull(name_expected)); assert_se(r == r_expected); assert_se(streq_ptr(name, name_expected)); } From e9edf285db966e92874b57098ba193277cef6121 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Zbigniew=20J=C4=99drzejewski-Szmek?= Date: Tue, 7 Jun 2022 15:33:22 +0200 Subject: [PATCH 3/8] test-dns-domain: a few more "NULL in printf %s"s --- src/test/test-dns-domain.c | 20 ++++++++++---------- 1 file changed, 10 insertions(+), 10 deletions(-) diff --git a/src/test/test-dns-domain.c b/src/test/test-dns-domain.c index a6e0c8501e..3a26ecaa70 100644 --- a/src/test/test-dns-domain.c +++ b/src/test/test-dns-domain.c @@ -61,15 +61,15 @@ static void test_dns_name_to_wire_format_one(const char *what, const char *expec uint8_t buffer[buffer_sz]; int r; - log_info("%s, %s, %zu, →%d", what, expect, buffer_sz, ret); + log_info("%s, %s, %zu, →%d", what, strnull(expect), buffer_sz, ret); r = dns_name_to_wire_format(what, buffer, buffer_sz, false); assert_se(r == ret); - if (r < 0) - return; - - assert_se(!memcmp(buffer, expect, r)); + if (r >= 0) { + assert(expect); /* for gcc */ + assert_se(memcmp(buffer, expect, r) == 0); + } } TEST(dns_name_to_wire_format) { @@ -165,7 +165,7 @@ static void test_dns_label_escape_one(const char *what, size_t l, const char *ex _cleanup_free_ char *t = NULL; int r; - log_info("%s, %zu, %s, →%d", what, l, expect, ret); + log_info("%s, %zu, %s, →%d", what, l, strnull(expect), ret); r = dns_label_escape_new(what, l, &t); assert_se(r == ret); @@ -483,7 +483,7 @@ TEST(dnssd_srv_type_is_valid) { static void test_dns_service_join_one(const char *a, const char *b, const char *c, int r, const char *d) { _cleanup_free_ char *x = NULL, *y = NULL, *z = NULL, *t = NULL; - log_info("%s, %s, %s, →%d, %s", a, b, c, r, d); + log_info("%s, %s, %s, →%d, %s", strnull(a), strnull(b), strnull(c), r, strnull(d)); assert_se(dns_service_join(a, b, c, &t) == r); assert_se(streq_ptr(t, d)); @@ -515,7 +515,7 @@ TEST(dns_service_join) { static void test_dns_service_split_one(const char *joined, const char *a, const char *b, const char *c, int r) { _cleanup_free_ char *x = NULL, *y = NULL, *z = NULL, *t = NULL; - log_info("%s, %s, %s, %s, →%d", joined, a, b, c, r); + log_info("%s, %s, %s, %s, →%d", joined, strnull(a), strnull(b), strnull(c), r); assert_se(dns_service_split(joined, &x, &y, &z) == r); assert_se(streq_ptr(x, a)); @@ -545,7 +545,7 @@ TEST(dns_service_split) { static void test_dns_name_change_suffix_one(const char *name, const char *old_suffix, const char *new_suffix, int r, const char *result) { _cleanup_free_ char *s = NULL; - log_info("%s, %s, %s, →%s", name, old_suffix, new_suffix, result); + log_info("%s, %s, %s, →%s", name, old_suffix, new_suffix, strnull(result)); assert_se(dns_name_change_suffix(name, old_suffix, new_suffix, &s) == r); assert_se(streq_ptr(s, result)); @@ -566,7 +566,7 @@ TEST(dns_name_change_suffix) { static void test_dns_name_suffix_one(const char *name, unsigned n_labels, const char *result, int ret) { const char *p = NULL; - log_info("%s, %d, →%s, %d", name, n_labels, result, ret); + log_info("%s, %d, →%s, %d", name, n_labels, strnull(result), ret); assert_se(ret == dns_name_suffix(name, n_labels, &p)); assert_se(streq_ptr(p, result)); From fa1aa468b941f7d89677ea09cd413f55b5c8c030 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Zbigniew=20J=C4=99drzejewski-Szmek?= Date: Tue, 7 Jun 2022 16:03:12 +0200 Subject: [PATCH 4/8] test-process-util: do not pass NULL to printf, simplify tests We don't need to fork to test that the function returns -EINVAL on null input. So let's simplify things a bit. --- src/test/test-process-util.c | 13 +++++++------ 1 file changed, 7 insertions(+), 6 deletions(-) diff --git a/src/test/test-process-util.c b/src/test/test-process-util.c index 7a8adad50c..990eb1e2b6 100644 --- a/src/test/test-process-util.c +++ b/src/test/test-process-util.c @@ -528,7 +528,7 @@ static void test_rename_process_now(const char *p, int ret) { _cleanup_free_ char *comm = NULL, *cmdline = NULL; int r; - log_info("/* %s */", __func__); + log_info("/* %s(%s) */", __func__, p); r = rename_process(p); assert_se(r == ret || @@ -574,7 +574,7 @@ static void test_rename_process_one(const char *p, int ret) { siginfo_t si; pid_t pid; - log_info("/* %s */", __func__); + log_info("/* %s(%s) */", __func__, p); pid = fork(); assert_se(pid >= 0); @@ -590,6 +590,11 @@ static void test_rename_process_one(const char *p, int ret) { assert_se(si.si_status == EXIT_SUCCESS); } +TEST(rename_process_invalid) { + assert_se(rename_process(NULL) == -EINVAL); + assert_se(rename_process("") == -EINVAL); +} + TEST(rename_process_multi) { pid_t pid; @@ -612,14 +617,10 @@ TEST(rename_process_multi) { (void) setresuid(99, 99, 99); /* change uid when running privileged */ test_rename_process_now("time!", 0); test_rename_process_now("0", 1); /* shorter than "one", should fit */ - test_rename_process_one("", -EINVAL); - test_rename_process_one(NULL, -EINVAL); _exit(EXIT_SUCCESS); } TEST(rename_process) { - test_rename_process_one(NULL, -EINVAL); - test_rename_process_one("", -EINVAL); test_rename_process_one("foo", 1); /* should always fit */ test_rename_process_one("this is a really really long process name, followed by some more words", 0); /* unlikely to fit */ test_rename_process_one("1234567", 1); /* should always fit */ From 482dac94317b290596b9ef5804a31ce368498d94 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Zbigniew=20J=C4=99drzejewski-Szmek?= Date: Tue, 7 Jun 2022 16:10:58 +0200 Subject: [PATCH 5/8] test-udev-util: do not pass NULL to printf, style fixes --- src/test/test-udev-util.c | 16 ++++++++-------- 1 file changed, 8 insertions(+), 8 deletions(-) diff --git a/src/test/test-udev-util.c b/src/test/test-udev-util.c index d413997189..35b20dc172 100644 --- a/src/test/test-udev-util.c +++ b/src/test/test-udev-util.c @@ -13,7 +13,7 @@ static void test_udev_rule_parse_value_one(const char *in, const char *expected_ char *value = UINT_TO_PTR(0x12345678U); char *endpos = UINT_TO_PTR(0x87654321U); - log_info("/* %s (%s, %s, %d) */", __func__, in, expected_value, expected_retval); + log_info("/* %s (%s, %s, %d) */", __func__, in, strnull(expected_value), expected_retval); assert_se(str = strdup(in)); assert_se(udev_rule_parse_value(str, &value, &endpos) == expected_retval); @@ -36,9 +36,9 @@ TEST(udev_rule_parse_value) { /* input: "va'l\'id\"op\"erand" * parsed: va'l\'id"op"erand */ test_udev_rule_parse_value_one("\"va'l\\'id\\\"op\\\"erand\"", "va'l\\'id\"op\"erand", 0); - test_udev_rule_parse_value_one("no quotes", 0, -EINVAL); + test_udev_rule_parse_value_one("no quotes", NULL, -EINVAL); test_udev_rule_parse_value_one("\"\\\\a\\b\\x\\y\"", "\\\\a\\b\\x\\y", 0); - test_udev_rule_parse_value_one("\"reject\0nul\"", 0, -EINVAL); + test_udev_rule_parse_value_one("\"reject\0nul\"", NULL, -EINVAL); /* input: e"" */ test_udev_rule_parse_value_one("e\"\"", "", 0); /* input: e"1234" */ @@ -46,13 +46,13 @@ TEST(udev_rule_parse_value) { /* input: e"\"" */ test_udev_rule_parse_value_one("e\"\\\"\"", "\"", 0); /* input: e"\ */ - test_udev_rule_parse_value_one("e\"\\", 0, -EINVAL); + test_udev_rule_parse_value_one("e\"\\", NULL, -EINVAL); /* input: e"\" */ - test_udev_rule_parse_value_one("e\"\\\"", 0, -EINVAL); + test_udev_rule_parse_value_one("e\"\\\"", NULL, -EINVAL); /* input: e"\\" */ test_udev_rule_parse_value_one("e\"\\\\\"", "\\", 0); /* input: e"\\\" */ - test_udev_rule_parse_value_one("e\"\\\\\\\"", 0, -EINVAL); + test_udev_rule_parse_value_one("e\"\\\\\\\"", NULL, -EINVAL); /* input: e"\\\"" */ test_udev_rule_parse_value_one("e\"\\\\\\\"\"", "\\\"", 0); /* input: e"\\\\" */ @@ -63,9 +63,9 @@ TEST(udev_rule_parse_value) { test_udev_rule_parse_value_one( "e\"single\\rcharacter\\t\\aescape\\bsequence\"", "single\rcharacter\t\aescape\bsequence", 0); /* input: e"reject\invalid escape sequence" */ - test_udev_rule_parse_value_one("e\"reject\\invalid escape sequence", 0, -EINVAL); + test_udev_rule_parse_value_one("e\"reject\\invalid escape sequence", NULL, -EINVAL); /* input: e"\ */ - test_udev_rule_parse_value_one("e\"\\", 0, -EINVAL); + test_udev_rule_parse_value_one("e\"\\", NULL, -EINVAL); /* input: "s\u1d1c\u1d04\u029c \u1d1c\u0274\u026a\u1d04\u1d0f\u1d05\u1d07 \U0001d568\U0001d560\U0001d568" */ test_udev_rule_parse_value_one( "e\"s\\u1d1c\\u1d04\\u029c \\u1d1c\\u0274\\u026a\\u1d04\\u1d0f\\u1d05\\u1d07 \\U0001d568\\U0001d560\\U0001d568\"", From 73e910926ef876948c9155b8e8c0b4a463900cbf Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Zbigniew=20J=C4=99drzejewski-Szmek?= Date: Tue, 7 Jun 2022 16:21:25 +0200 Subject: [PATCH 6/8] shared/bus-util: simplification --- src/shared/bus-util.c | 26 +++++++++----------------- 1 file changed, 9 insertions(+), 17 deletions(-) diff --git a/src/shared/bus-util.c b/src/shared/bus-util.c index a907b67a70..ea506f2725 100644 --- a/src/shared/bus-util.c +++ b/src/shared/bus-util.c @@ -323,8 +323,6 @@ int bus_connect_transport( } int bus_connect_transport_systemd(BusTransport transport, const char *host, bool user, sd_bus **bus) { - int r; - assert(transport >= 0); assert(transport < _BUS_TRANSPORT_MAX); assert(bus); @@ -336,29 +334,23 @@ int bus_connect_transport_systemd(BusTransport transport, const char *host, bool case BUS_TRANSPORT_LOCAL: if (user) - r = bus_connect_user_systemd(bus); - else { - if (sd_booted() <= 0) - /* Print a friendly message when the local system is actually not running systemd as PID 1. */ - return log_error_errno(SYNTHETIC_ERRNO(EHOSTDOWN), - "System has not been booted with systemd as init system (PID 1). Can't operate."); - r = bus_connect_system_systemd(bus); - } - break; + return bus_connect_user_systemd(bus); + + if (sd_booted() <= 0) + /* Print a friendly message when the local system is actually not running systemd as PID 1. */ + return log_error_errno(SYNTHETIC_ERRNO(EHOSTDOWN), + "System has not been booted with systemd as init system (PID 1). Can't operate."); + return bus_connect_system_systemd(bus); case BUS_TRANSPORT_REMOTE: - r = sd_bus_open_system_remote(bus, host); - break; + return sd_bus_open_system_remote(bus, host); case BUS_TRANSPORT_MACHINE: - r = sd_bus_open_system_machine(bus, host); - break; + return sd_bus_open_system_machine(bus, host); default: assert_not_reached(); } - - return r; } /** From c90821aa49e918473c873b93108c8e6755074b27 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Zbigniew=20J=C4=99drzejewski-Szmek?= Date: Tue, 7 Jun 2022 16:23:36 +0200 Subject: [PATCH 7/8] shared/bus-util: rename variables to follow newer style --- src/shared/bus-util.c | 22 ++++++++++------------ src/shared/bus-util.h | 4 ++-- 2 files changed, 12 insertions(+), 14 deletions(-) diff --git a/src/shared/bus-util.c b/src/shared/bus-util.c index ea506f2725..e12f95d4b1 100644 --- a/src/shared/bus-util.c +++ b/src/shared/bus-util.c @@ -197,14 +197,14 @@ int bus_check_peercred(sd_bus *c) { return 1; } -int bus_connect_system_systemd(sd_bus **_bus) { +int bus_connect_system_systemd(sd_bus **ret_bus) { _cleanup_(sd_bus_close_unrefp) sd_bus *bus = NULL; int r; - assert(_bus); + assert(ret_bus); if (geteuid() != 0) - return sd_bus_default_system(_bus); + return sd_bus_default_system(ret_bus); /* If we are root then let's talk directly to the system * instance, instead of going via the bus */ @@ -219,28 +219,27 @@ int bus_connect_system_systemd(sd_bus **_bus) { r = sd_bus_start(bus); if (r < 0) - return sd_bus_default_system(_bus); + return sd_bus_default_system(ret_bus); r = bus_check_peercred(bus); if (r < 0) return r; - *_bus = TAKE_PTR(bus); - + *ret_bus = TAKE_PTR(bus); return 0; } -int bus_connect_user_systemd(sd_bus **_bus) { +int bus_connect_user_systemd(sd_bus **ret_bus) { _cleanup_(sd_bus_close_unrefp) sd_bus *bus = NULL; _cleanup_free_ char *ee = NULL; const char *e; int r; - assert(_bus); + assert(ret_bus); e = secure_getenv("XDG_RUNTIME_DIR"); if (!e) - return sd_bus_default_user(_bus); + return sd_bus_default_user(ret_bus); ee = bus_address_escape(e); if (!ee) @@ -256,14 +255,13 @@ int bus_connect_user_systemd(sd_bus **_bus) { r = sd_bus_start(bus); if (r < 0) - return sd_bus_default_user(_bus); + return sd_bus_default_user(ret_bus); r = bus_check_peercred(bus); if (r < 0) return r; - *_bus = TAKE_PTR(bus); - + *ret_bus = TAKE_PTR(bus); return 0; } diff --git a/src/shared/bus-util.h b/src/shared/bus-util.h index 00514e359f..955cdcb57c 100644 --- a/src/shared/bus-util.h +++ b/src/shared/bus-util.h @@ -33,8 +33,8 @@ bool bus_error_is_unknown_service(const sd_bus_error *error); int bus_check_peercred(sd_bus *c); -int bus_connect_system_systemd(sd_bus **_bus); -int bus_connect_user_systemd(sd_bus **_bus); +int bus_connect_system_systemd(sd_bus **ret_bus); +int bus_connect_user_systemd(sd_bus **ret_bus); int bus_connect_transport(BusTransport transport, const char *host, bool user, sd_bus **bus); int bus_connect_transport_systemd(BusTransport transport, const char *host, bool user, sd_bus **bus); From c4da8e90fa65a042c6cdf495f5bfe5df087010a4 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Zbigniew=20J=C4=99drzejewski-Szmek?= Date: Tue, 7 Jun 2022 16:26:18 +0200 Subject: [PATCH 8/8] systemctl: simplify code a bit gcc insists that bus may be used unitialized here, but I don't see any possibility of that. --- src/systemctl/systemctl-util.c | 9 +++++---- 1 file changed, 5 insertions(+), 4 deletions(-) diff --git a/src/systemctl/systemctl-util.c b/src/systemctl/systemctl-util.c index 6a71bce5fb..13a039cdfb 100644 --- a/src/systemctl/systemctl-util.c +++ b/src/systemctl/systemctl-util.c @@ -291,8 +291,6 @@ int check_triggering_units(sd_bus *bus, const char *unit) { _cleanup_(sd_bus_error_free) sd_bus_error error = SD_BUS_ERROR_NULL; _cleanup_free_ char *n = NULL, *dbus_path = NULL, *load_state = NULL; _cleanup_strv_free_ char **triggered_by = NULL; - bool print_warning_label = true; - UnitActiveState active_state; int r; r = unit_name_mangle(unit, 0, &n); @@ -321,7 +319,10 @@ int check_triggering_units(sd_bus *bus, const char *unit) { if (r < 0) return log_error_errno(r, "Failed to get triggered by array of %s: %s", n, bus_error_message(&error, r)); + bool first = true; STRV_FOREACH(i, triggered_by) { + UnitActiveState active_state; + r = get_state_one_unit(bus, *i, &active_state); if (r < 0) return r; @@ -329,9 +330,9 @@ int check_triggering_units(sd_bus *bus, const char *unit) { if (!IN_SET(active_state, UNIT_ACTIVE, UNIT_RELOADING)) continue; - if (print_warning_label) { + if (first) { log_warning("Warning: Stopping %s, but it can still be activated by:", n); - print_warning_label = false; + first = false; } log_warning(" %s", *i);