From 72420d39d7e3edefb1973395a9918d1d6509b168 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Zbigniew=20J=C4=99drzejewski-Szmek?= Date: Sun, 26 Nov 2023 15:54:11 +0100 Subject: [PATCH 1/4] test-macro: use capital test names for macro tests It's just easier if the test is named as the thing being tested. Also, this way inconsistent, because lower in the file uppercase test names are used. --- src/test/test-macro.c | 18 +++++++++--------- 1 file changed, 9 insertions(+), 9 deletions(-) diff --git a/src/test/test-macro.c b/src/test/test-macro.c index 63fe6ea95e..b91a1f96ac 100644 --- a/src/test/test-macro.c +++ b/src/test/test-macro.c @@ -15,7 +15,7 @@ TEST(saturate_add) { assert_se(saturate_add(60, 60, 50) == 50); } -TEST(align_power2) { +TEST(ALIGN_POWER2) { unsigned long i, p2; assert_se(ALIGN_POWER2(0) == 0); @@ -57,7 +57,7 @@ TEST(align_power2) { } } -TEST(max) { +TEST(MAX) { static const struct { int a; int b[CONST_MAX(10, 100)]; @@ -159,7 +159,7 @@ TEST(container_of) { #pragma GCC diagnostic pop -TEST(div_round_up) { +TEST(DIV_ROUND_UP) { int div; /* basic tests */ @@ -192,7 +192,7 @@ TEST(div_round_up) { assert_se(0xfffffffdU / 10U + !!(0xfffffffdU % 10U) == 429496730U); } -TEST(ptr_to_int) { +TEST(PTR_TO_INT) { /* Primary reason to have this test is to validate that pointers are large enough to hold entire int range */ assert_se(PTR_TO_INT(INT_TO_PTR(0)) == 0); assert_se(PTR_TO_INT(INT_TO_PTR(1)) == 1); @@ -201,7 +201,7 @@ TEST(ptr_to_int) { assert_se(PTR_TO_INT(INT_TO_PTR(INT_MIN)) == INT_MIN); } -TEST(in_set) { +TEST(IN_SET) { assert_se(IN_SET(1, 1, 2)); assert_se(IN_SET(1, 1, 2, 3, 4)); assert_se(IN_SET(2, 1, 2, 3, 4)); @@ -221,7 +221,7 @@ TEST(in_set) { assert_se(!IN_SET(t.x, 2, 3, 4)); } -TEST(foreach_pointer) { +TEST(FOREACH_POINTER) { int a, b, c, *i; size_t k = 0; @@ -300,7 +300,7 @@ TEST(foreach_pointer) { assert_se(k == 11); } -TEST(foreach_va_args) { +TEST(FOREACH_VA_ARGS) { size_t i; i = 0; @@ -484,7 +484,7 @@ TEST(foreach_va_args) { assert_se(false); } -TEST(align_to) { +TEST(ALIGN_TO) { assert_se(ALIGN_TO(0, 1) == 0); assert_se(ALIGN_TO(1, 1) == 1); assert_se(ALIGN_TO(2, 1) == 2); @@ -1005,7 +1005,7 @@ TEST(FOREACH_ARRAY) { assert_se(ROUND_UP(x, y) == max_value); \ }) -TEST(round_up) { +TEST(ROUND_UP) { TEST_ROUND_UP_BY_TYPE(uint8_t, UINT8_MAX); TEST_ROUND_UP_BY_TYPE(uint16_t, UINT16_MAX); TEST_ROUND_UP_BY_TYPE(uint32_t, UINT32_MAX); From cffa43255a4fdfa3611ded5e8b6f2305a17b3ba7 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Zbigniew=20J=C4=99drzejewski-Szmek?= Date: Wed, 29 Nov 2023 14:01:13 +0100 Subject: [PATCH 2/4] run: adjust indentation --- src/run/run.c | 13 +++++++++---- 1 file changed, 9 insertions(+), 4 deletions(-) diff --git a/src/run/run.c b/src/run/run.c index f97150eaab..5c0f3ad247 100644 --- a/src/run/run.c +++ b/src/run/run.c @@ -1376,7 +1376,10 @@ static int start_transient_service(sd_bus *bus) { if (r < 0) return bus_log_parse_error(r); - r = bus_wait_for_jobs_one(w, object, arg_quiet, arg_runtime_scope == RUNTIME_SCOPE_USER ? STRV_MAKE_CONST("--user") : NULL); + r = bus_wait_for_jobs_one(w, + object, + arg_quiet, + arg_runtime_scope == RUNTIME_SCOPE_USER ? STRV_MAKE_CONST("--user") : NULL); if (r < 0) return r; } @@ -1960,9 +1963,11 @@ static int run(int argc, char* argv[]) { " Use --expand-environment=yes/no to explicitly control it as needed."); } - /* If --wait is used connect via the bus, unconditionally, as ref/unref is not supported via the limited direct - * connection */ - if (arg_wait || arg_stdio != ARG_STDIO_NONE || (arg_runtime_scope == RUNTIME_SCOPE_USER && arg_transport != BUS_TRANSPORT_LOCAL)) + /* If --wait is used connect via the bus, unconditionally, as ref/unref is not supported via the + * limited direct connection */ + if (arg_wait || + arg_stdio != ARG_STDIO_NONE || + (arg_runtime_scope == RUNTIME_SCOPE_USER && arg_transport != BUS_TRANSPORT_LOCAL)) r = bus_connect_transport(arg_transport, arg_host, arg_runtime_scope, &bus); else r = bus_connect_transport_systemd(arg_transport, arg_host, arg_runtime_scope, &bus); From 9ce3440a6fb3fb785e0fa3a6fcf60af7ef652440 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Zbigniew=20J=C4=99drzejewski-Szmek?= Date: Wed, 29 Nov 2023 14:13:33 +0100 Subject: [PATCH 3/4] run: fix bad escaping and memory ownership confusion arg_description was either set to arg_unit (i.e. a const char*), or to char *description, the result of allocation in run(). But description was decorated with _cleanup_, so it would be freed when going out of the function. Nothing bad would happen, because the program would exit after exiting from run(), but this is just all too messy. Also, strv_join(" ") + shell_escape() is not a good way to escape command lines. In particular, one the join has happened, we cannot distinguish empty arguments, or arguments with whitespace, etc. We have a helper function to do the escaping properly, so let's use that. Fixup for 2c29813da3421b77eca5e5cdc3b9a863cad473b9. --- src/run/run.c | 27 +++++++++++++-------------- 1 file changed, 13 insertions(+), 14 deletions(-) diff --git a/src/run/run.c b/src/run/run.c index 5c0f3ad247..88eca0fd6d 100644 --- a/src/run/run.c +++ b/src/run/run.c @@ -43,7 +43,7 @@ static bool arg_remain_after_exit = false; static bool arg_no_block = false; static bool arg_wait = false; static const char *arg_unit = NULL; -static const char *arg_description = NULL; +static char *arg_description = NULL; static const char *arg_slice = NULL; static bool arg_slice_inherit = false; static int arg_expand_environment = -1; @@ -74,6 +74,7 @@ static char *arg_working_directory = NULL; static bool arg_shell = false; static char **arg_cmdline = NULL; +STATIC_DESTRUCTOR_REGISTER(arg_description, freep); STATIC_DESTRUCTOR_REGISTER(arg_environment, strv_freep); STATIC_DESTRUCTOR_REGISTER(arg_property, strv_freep); STATIC_DESTRUCTOR_REGISTER(arg_path_property, strv_freep); @@ -281,7 +282,9 @@ static int parse_argv(int argc, char *argv[]) { break; case ARG_DESCRIPTION: - arg_description = optarg; + r = free_and_strdup(&arg_description, optarg); + if (r < 0) + return r; break; case ARG_SLICE: @@ -1911,7 +1914,6 @@ static bool shall_make_executable_absolute(void) { static int run(int argc, char* argv[]) { _cleanup_(sd_bus_flush_close_unrefp) sd_bus *bus = NULL; - _cleanup_free_ char *description = NULL; int r; log_show_color(true); @@ -1936,19 +1938,16 @@ static int run(int argc, char* argv[]) { } if (!arg_description) { + char *t; + if (strv_isempty(arg_cmdline)) - arg_description = arg_unit; - else { - _cleanup_free_ char *joined = strv_join(arg_cmdline, " "); - if (!joined) - return log_oom(); + t = strdup(arg_unit); + else + t = quote_command_line(arg_cmdline, SHELL_ESCAPE_EMPTY); + if (!t) + return log_oom(); - description = shell_escape(joined, "\""); - if (!description) - return log_oom(); - - arg_description = description; - } + free_and_replace(arg_description, t); } /* For backward compatibility reasons env var expansion is disabled by default for scopes, and From ccd31de8bf4717f0dd889c97440b4037909eb3bc Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Zbigniew=20J=C4=99drzejewski-Szmek?= Date: Wed, 29 Nov 2023 14:38:06 +0100 Subject: [PATCH 4/4] stdio-bridge: return immediately if we can Follow-up for 0321248b79d14ceddd36140b327332f145ae68e7. --- src/stdio-bridge/stdio-bridge.c | 10 ++++------ 1 file changed, 4 insertions(+), 6 deletions(-) diff --git a/src/stdio-bridge/stdio-bridge.c b/src/stdio-bridge/stdio-bridge.c index ce2bd5f86f..fe551cfaea 100644 --- a/src/stdio-bridge/stdio-bridge.c +++ b/src/stdio-bridge/stdio-bridge.c @@ -183,12 +183,12 @@ static int run(int argc, char *argv[]) { r = sd_bus_process(a, &m); if (ERRNO_IS_NEG_DISCONNECT(r)) /* Treat 'connection reset by peer' as clean exit condition */ - break; + return 0; if (r < 0) return log_error_errno(r, "Failed to process bus a: %m"); if (m) { if (sd_bus_message_is_signal(m, "org.freedesktop.DBus.Local", "Disconnected")) - break; + return 0; r = sd_bus_send(b, m, NULL); if (r < 0) @@ -200,12 +200,12 @@ static int run(int argc, char *argv[]) { r = sd_bus_process(b, &m); if (ERRNO_IS_NEG_DISCONNECT(r)) /* Treat 'connection reset by peer' as clean exit condition */ - break; + return 0; if (r < 0) return log_error_errno(r, "Failed to process bus: %m"); if (m) { if (sd_bus_message_is_signal(m, "org.freedesktop.DBus.Local", "Disconnected")) - break; + return 0; r = sd_bus_send(a, m, NULL); if (r < 0) @@ -247,8 +247,6 @@ static int run(int argc, char *argv[]) { if (r < 0 && !ERRNO_IS_TRANSIENT(r)) /* don't be bothered by signals, i.e. EINTR */ return log_error_errno(r, "ppoll() failed: %m"); } - - return 0; } DEFINE_MAIN_FUNCTION(run);