From 953ff3c83e95ae3943e4307b64fe429d56cc8ac6 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Zbigniew=20J=C4=99drzejewski-Szmek?= Date: Thu, 14 May 2026 10:35:30 +0200 Subject: [PATCH 1/8] core: reorder cases in parse_argv() to match order in --help MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit The hidden-from-help options (--crash-reboot, --service-watchdogs, --deserialize, --switched-root, --machine-id, -D, -b/-s/-z, ?) move to the bottom. The 'b'/'s'/'z' → '?' fall-through is preserved. Co-developed-by: Claude Opus 4.7 --- src/core/main.c | 180 ++++++++++++++++++++++++------------------------ 1 file changed, 91 insertions(+), 89 deletions(-) diff --git a/src/core/main.c b/src/core/main.c index c10df7d87a..dc55ea62e9 100644 --- a/src/core/main.c +++ b/src/core/main.c @@ -990,11 +990,90 @@ static int parse_argv(int argc, char *argv[]) { switch (c) { - case ARG_LOG_LEVEL: - r = log_set_max_level_from_string(optarg); - if (r < 0) - return log_error_errno(r, "Failed to parse log level \"%s\": %m", optarg); + case 'h': + arg_action = ACTION_HELP; + break; + case ARG_VERSION: + arg_action = ACTION_VERSION; + break; + + case ARG_TEST: + arg_action = ACTION_TEST; + break; + + case ARG_SYSTEM: + arg_runtime_scope = RUNTIME_SCOPE_SYSTEM; + break; + + case ARG_USER: + arg_runtime_scope = RUNTIME_SCOPE_USER; + user_arg_seen = true; + break; + + case ARG_DUMP_CONFIGURATION_ITEMS: + arg_action = ACTION_DUMP_CONFIGURATION_ITEMS; + break; + + case ARG_DUMP_BUS_PROPERTIES: + arg_action = ACTION_DUMP_BUS_PROPERTIES; + break; + + case ARG_BUS_INTROSPECT: + arg_bus_introspect = optarg; + arg_action = ACTION_BUS_INTROSPECT; + break; + + case ARG_UNIT: + r = free_and_strdup(&arg_default_unit, optarg); + if (r < 0) + return log_error_errno(r, "Failed to set default unit \"%s\": %m", optarg); + + break; + + case ARG_DUMP_CORE: + r = parse_boolean_argument("--dump-core", optarg, &arg_dump_core); + if (r < 0) + return r; + break; + + case ARG_CRASH_CHVT: + r = parse_crash_chvt(optarg, &arg_crash_chvt); + if (r < 0) + return log_error_errno(r, "Failed to parse crash virtual terminal index: \"%s\": %m", + optarg); + break; + + case ARG_CRASH_ACTION: + r = crash_action_from_string(optarg); + if (r < 0) + return log_error_errno(r, "Failed to parse crash action \"%s\": %m", optarg); + arg_crash_action = r; + break; + + case ARG_CRASH_SHELL: + r = parse_boolean_argument("--crash-shell", optarg, &arg_crash_shell); + if (r < 0) + return r; + break; + + case ARG_CONFIRM_SPAWN: + arg_confirm_spawn = mfree(arg_confirm_spawn); + + r = parse_confirm_spawn(optarg, &arg_confirm_spawn); + if (r < 0) + return log_error_errno(r, "Failed to parse confirm spawn option: \"%s\": %m", + optarg); + break; + + case ARG_SHOW_STATUS: + if (optarg) { + r = parse_show_status(optarg, &arg_show_status); + if (r < 0) + return log_error_errno(r, "Failed to parse show status boolean: \"%s\": %m", + optarg); + } else + arg_show_status = SHOW_STATUS_YES; break; case ARG_LOG_TARGET: @@ -1004,6 +1083,13 @@ static int parse_argv(int argc, char *argv[]) { break; + case ARG_LOG_LEVEL: + r = log_set_max_level_from_string(optarg); + if (r < 0) + return log_error_errno(r, "Failed to parse log level \"%s\": %m", optarg); + + break; + case ARG_LOG_COLOR: if (optarg) { @@ -1055,65 +1141,11 @@ static int parse_argv(int argc, char *argv[]) { arg_defaults.std_error = r; break; - case ARG_UNIT: - r = free_and_strdup(&arg_default_unit, optarg); - if (r < 0) - return log_error_errno(r, "Failed to set default unit \"%s\": %m", optarg); - - break; - - case ARG_SYSTEM: - arg_runtime_scope = RUNTIME_SCOPE_SYSTEM; - break; - - case ARG_USER: - arg_runtime_scope = RUNTIME_SCOPE_USER; - user_arg_seen = true; - break; - - case ARG_TEST: - arg_action = ACTION_TEST; - break; - case ARG_NO_PAGER: arg_pager_flags |= PAGER_DISABLE; break; - case ARG_VERSION: - arg_action = ACTION_VERSION; - break; - - case ARG_DUMP_CONFIGURATION_ITEMS: - arg_action = ACTION_DUMP_CONFIGURATION_ITEMS; - break; - - case ARG_DUMP_BUS_PROPERTIES: - arg_action = ACTION_DUMP_BUS_PROPERTIES; - break; - - case ARG_BUS_INTROSPECT: - arg_bus_introspect = optarg; - arg_action = ACTION_BUS_INTROSPECT; - break; - - case ARG_DUMP_CORE: - r = parse_boolean_argument("--dump-core", optarg, &arg_dump_core); - if (r < 0) - return r; - break; - - case ARG_CRASH_CHVT: - r = parse_crash_chvt(optarg, &arg_crash_chvt); - if (r < 0) - return log_error_errno(r, "Failed to parse crash virtual terminal index: \"%s\": %m", - optarg); - break; - - case ARG_CRASH_SHELL: - r = parse_boolean_argument("--crash-shell", optarg, &arg_crash_shell); - if (r < 0) - return r; - break; + /* Options not shown in --help. */ case ARG_CRASH_REBOOT: r = parse_boolean_argument("--crash-reboot", optarg, NULL); @@ -1122,38 +1154,12 @@ static int parse_argv(int argc, char *argv[]) { arg_crash_action = r > 0 ? CRASH_REBOOT : CRASH_FREEZE; break; - case ARG_CRASH_ACTION: - r = crash_action_from_string(optarg); - if (r < 0) - return log_error_errno(r, "Failed to parse crash action \"%s\": %m", optarg); - arg_crash_action = r; - break; - - case ARG_CONFIRM_SPAWN: - arg_confirm_spawn = mfree(arg_confirm_spawn); - - r = parse_confirm_spawn(optarg, &arg_confirm_spawn); - if (r < 0) - return log_error_errno(r, "Failed to parse confirm spawn option: \"%s\": %m", - optarg); - break; - case ARG_SERVICE_WATCHDOGS: r = parse_boolean_argument("--service-watchdogs=", optarg, &arg_service_watchdogs); if (r < 0) return r; break; - case ARG_SHOW_STATUS: - if (optarg) { - r = parse_show_status(optarg, &arg_show_status); - if (r < 0) - return log_error_errno(r, "Failed to parse show status boolean: \"%s\": %m", - optarg); - } else - arg_show_status = SHOW_STATUS_YES; - break; - case ARG_DESERIALIZE: { int fd; FILE *f; @@ -1184,10 +1190,6 @@ static int parse_argv(int argc, char *argv[]) { return log_error_errno(r, "MachineID '%s' is not valid: %m", optarg); break; - case 'h': - arg_action = ACTION_HELP; - break; - case 'D': log_set_max_level(LOG_DEBUG); break; From 2289ac82d4150174212e716b696b7bddb4f240d7 Mon Sep 17 00:00:00 2001 From: Lennart Poettering Date: Thu, 14 May 2026 17:56:56 +0200 Subject: [PATCH 2/8] shared/options: introduce OPTION_ERROR [zjs: this was originally proposed in https://github.com/systemd/systemd/commit/930fc9d6980f27b278527b0d6117f97296fcaf6a. I'm rescuing one chunk from that patch.] --- src/shared/options.h | 4 ++++ 1 file changed, 4 insertions(+) diff --git a/src/shared/options.h b/src/shared/options.h index d60c9afe81..a6c55e0bdc 100644 --- a/src/shared/options.h +++ b/src/shared/options.h @@ -80,6 +80,10 @@ typedef struct Option { #define OPTION_POSITIONAL OPTION_FULL(OPTION_POSITIONAL_ENTRY, /* sc= */ 0, "(positional)", /* mv= */ NULL, /* h= */ NULL) #define OPTION_HELP_VERBATIM(lc, h) OPTION_FULL(OPTION_HELP_ENTRY_VERBATIM, /* sc= */ 0, lc, /* mv= */ NULL, h) +/* This can be used when custom error handling is needed. */ +#define OPTION_ERROR \ + case INT_MIN ... -1 + #define OPTION_COMMON_HELP \ OPTION('h', "help", NULL, "Show this help") From 5597a78df2618e64ccdda4dcd2e7e4942903f2ca Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Zbigniew=20J=C4=99drzejewski-Szmek?= Date: Thu, 14 May 2026 10:38:35 +0200 Subject: [PATCH 3/8] core: convert PID 1 parse_argv to OPTION macros parse_argv() uses FOREACH_OPTION (not _OR_RETURN) so we can preserve the existing PID 1 tolerance: an unknown option, or one of the sysvinit-style -b/-s/-z catch-alls, returns 0 instead of -EINVAL when getpid_cached() == 1. The docs documented --crash-vt=, but the code implemented "crash-chvt", as introduced in b9e74c399458a1146894ce371e7d85c60658110c. The output from --help is now modified to match code. getopt-defs.h is intentionally left in place since proc_cmdline_filter_pid1_args() in src/basic/proc-cmdline.c still uses its COMMON_/SYSTEMD_/SHUTDOWN_GETOPT_* macros to walk the kernel command line. Previously, opterr=0 was used to suppress error messages about option parsing in PID1. They are now logged at debug level (if debug logging is enabled.) Co-developed-by: Claude Opus 4.7 --- src/core/main.c | 258 ++++++++++++++++++++---------------------------- 1 file changed, 105 insertions(+), 153 deletions(-) diff --git a/src/core/main.c b/src/core/main.c index dc55ea62e9..d990017ded 100644 --- a/src/core/main.c +++ b/src/core/main.c @@ -1,7 +1,6 @@ /* SPDX-License-Identifier: LGPL-2.1-or-later */ #include -#include #include #include #include @@ -19,6 +18,7 @@ #include "sd-messages.h" #include "alloc-util.h" +#include "ansi-color.h" #include "apparmor-setup.h" #include "architecture.h" #include "argv-util.h" @@ -47,8 +47,10 @@ #include "fd-util.h" #include "fdset.h" #include "fileio.h" +#include "format-table.h" #include "format-util.h" -#include "getopt-defs.h" +#include "glyph-util.h" +#include "help-util.h" #include "hexdecoct.h" #include "hostname-setup.h" #include "id128-util.h" @@ -73,13 +75,13 @@ #include "mkdir-label.h" #include "mount-setup.h" #include "mount-util.h" +#include "options.h" #include "os-util.h" #include "osc-context.h" #include "pager.h" #include "parse-argument.h" #include "parse-util.h" #include "path-util.h" -#include "pretty-print.h" #include "proc-cmdline.h" #include "process-util.h" #include "random-util.h" @@ -966,251 +968,227 @@ static void set_manager_settings(Manager *m) { } static int parse_argv(int argc, char *argv[]) { - enum { - COMMON_GETOPT_ARGS, - SYSTEMD_GETOPT_ARGS, - }; - - static const struct option options[] = { - COMMON_GETOPT_OPTIONS, - SYSTEMD_GETOPT_OPTIONS, - {} - }; - - int c, r; bool user_arg_seen = false; + int r; assert(argc >= 1); assert(argv); - if (getpid_cached() == 1) - opterr = 0; - - while ((c = getopt_long(argc, argv, SYSTEMD_GETOPT_SHORT_OPTIONS, options, NULL)) >= 0) + int log_level_shift = getpid_cached() == 1 ? LOG_DEBUG - LOG_ERR : 0; + OptionParser opts = { argc, argv, .log_level_shift = log_level_shift }; + FOREACH_OPTION(c, &opts) switch (c) { - case 'h': + OPTION_COMMON_HELP: arg_action = ACTION_HELP; break; - case ARG_VERSION: + OPTION_COMMON_VERSION: arg_action = ACTION_VERSION; break; - case ARG_TEST: + OPTION_LONG("test", NULL, "Determine initial transaction, dump it and exit"): arg_action = ACTION_TEST; break; - case ARG_SYSTEM: + OPTION_LONG("system", NULL, "Combined with --test: operate in system mode"): arg_runtime_scope = RUNTIME_SCOPE_SYSTEM; break; - case ARG_USER: + OPTION_LONG("user", NULL, "Combined with --test: operate in user mode"): arg_runtime_scope = RUNTIME_SCOPE_USER; user_arg_seen = true; break; - case ARG_DUMP_CONFIGURATION_ITEMS: + OPTION_LONG("dump-configuration-items", NULL, "Dump understood unit configuration items"): arg_action = ACTION_DUMP_CONFIGURATION_ITEMS; break; - case ARG_DUMP_BUS_PROPERTIES: + OPTION_LONG("dump-bus-properties", NULL, "Dump exposed bus properties"): arg_action = ACTION_DUMP_BUS_PROPERTIES; break; - case ARG_BUS_INTROSPECT: - arg_bus_introspect = optarg; + OPTION_LONG("bus-introspect", "PATH", "Write XML introspection data"): + arg_bus_introspect = opts.arg; arg_action = ACTION_BUS_INTROSPECT; break; - case ARG_UNIT: - r = free_and_strdup(&arg_default_unit, optarg); + OPTION_LONG("unit", "UNIT", "Set default unit"): + r = free_and_strdup(&arg_default_unit, opts.arg); if (r < 0) - return log_error_errno(r, "Failed to set default unit \"%s\": %m", optarg); - + return log_error_errno(r, "Failed to set default unit \"%s\": %m", opts.arg); break; - case ARG_DUMP_CORE: - r = parse_boolean_argument("--dump-core", optarg, &arg_dump_core); + OPTION_LONG_FLAGS(OPTION_OPTIONAL_ARG, "dump-core", "BOOL", "Dump core on crash"): + r = parse_boolean_argument("--dump-core", opts.arg, &arg_dump_core); if (r < 0) return r; break; - case ARG_CRASH_CHVT: - r = parse_crash_chvt(optarg, &arg_crash_chvt); + OPTION_LONG("crash-chvt", "NR", "Change to specified VT on crash"): + r = parse_crash_chvt(opts.arg, &arg_crash_chvt); if (r < 0) return log_error_errno(r, "Failed to parse crash virtual terminal index: \"%s\": %m", - optarg); + opts.arg); break; - case ARG_CRASH_ACTION: - r = crash_action_from_string(optarg); + OPTION_LONG("crash-action", "ACTION", "Specify what to do on crash"): + r = crash_action_from_string(opts.arg); if (r < 0) - return log_error_errno(r, "Failed to parse crash action \"%s\": %m", optarg); + return log_error_errno(r, "Failed to parse crash action \"%s\": %m", opts.arg); arg_crash_action = r; break; - case ARG_CRASH_SHELL: - r = parse_boolean_argument("--crash-shell", optarg, &arg_crash_shell); + OPTION_LONG_FLAGS(OPTION_OPTIONAL_ARG, "crash-shell", "BOOL", "Run shell on crash"): + r = parse_boolean_argument("--crash-shell", opts.arg, &arg_crash_shell); if (r < 0) return r; break; - case ARG_CONFIRM_SPAWN: + OPTION_LONG_FLAGS(OPTION_OPTIONAL_ARG, "confirm-spawn", "BOOL", + "Ask for confirmation when spawning processes"): arg_confirm_spawn = mfree(arg_confirm_spawn); - r = parse_confirm_spawn(optarg, &arg_confirm_spawn); + r = parse_confirm_spawn(opts.arg, &arg_confirm_spawn); if (r < 0) return log_error_errno(r, "Failed to parse confirm spawn option: \"%s\": %m", - optarg); + opts.arg); break; - case ARG_SHOW_STATUS: - if (optarg) { - r = parse_show_status(optarg, &arg_show_status); + OPTION_LONG_FLAGS(OPTION_OPTIONAL_ARG, "show-status", "BOOL", + "Show status updates on the console during boot"): + if (opts.arg) { + r = parse_show_status(opts.arg, &arg_show_status); if (r < 0) return log_error_errno(r, "Failed to parse show status boolean: \"%s\": %m", - optarg); + opts.arg); } else arg_show_status = SHOW_STATUS_YES; break; - case ARG_LOG_TARGET: - r = log_set_target_from_string(optarg); + OPTION_LONG("log-target", "TARGET", + "Set log target (console, journal, kmsg, journal-or-kmsg, null)"): + r = log_set_target_from_string(opts.arg); if (r < 0) - return log_error_errno(r, "Failed to parse log target \"%s\": %m", optarg); - + return log_error_errno(r, "Failed to parse log target \"%s\": %m", opts.arg); break; - case ARG_LOG_LEVEL: - r = log_set_max_level_from_string(optarg); + OPTION_LONG("log-level", "LEVEL", + "Set log level (debug, info, notice, warning, err, crit, alert, emerg)"): + r = log_set_max_level_from_string(opts.arg); if (r < 0) - return log_error_errno(r, "Failed to parse log level \"%s\": %m", optarg); - + return log_error_errno(r, "Failed to parse log level \"%s\": %m", opts.arg); break; - case ARG_LOG_COLOR: - - if (optarg) { - r = log_show_color_from_string(optarg); + OPTION_LONG_FLAGS(OPTION_OPTIONAL_ARG, "log-color", "BOOL", "Highlight important log messages"): + if (opts.arg) { + r = log_show_color_from_string(opts.arg); if (r < 0) return log_error_errno(r, "Failed to parse log color setting \"%s\": %m", - optarg); + opts.arg); } else log_show_color(true); - break; - case ARG_LOG_LOCATION: - if (optarg) { - r = log_show_location_from_string(optarg); + OPTION_LONG_FLAGS(OPTION_OPTIONAL_ARG, "log-location", "BOOL", + "Include code location in log messages"): + if (opts.arg) { + r = log_show_location_from_string(opts.arg); if (r < 0) return log_error_errno(r, "Failed to parse log location setting \"%s\": %m", - optarg); + opts.arg); } else log_show_location(true); - break; - case ARG_LOG_TIME: - - if (optarg) { - r = log_show_time_from_string(optarg); + OPTION_LONG_FLAGS(OPTION_OPTIONAL_ARG, "log-time", "BOOL", "Prefix log messages with current time"): + if (opts.arg) { + r = log_show_time_from_string(opts.arg); if (r < 0) return log_error_errno(r, "Failed to parse log time setting \"%s\": %m", - optarg); + opts.arg); } else log_show_time(true); - break; - case ARG_DEFAULT_STD_OUTPUT: - r = exec_output_from_string(optarg); + OPTION_LONG("default-standard-output", "…", "Set default standard output for services"): + r = exec_output_from_string(opts.arg); if (r < 0) return log_error_errno(r, "Failed to parse default standard output setting \"%s\": %m", - optarg); + opts.arg); arg_defaults.std_output = r; break; - case ARG_DEFAULT_STD_ERROR: - r = exec_output_from_string(optarg); + OPTION_LONG("default-standard-error", "…", "Set default standard error output for services"): + r = exec_output_from_string(opts.arg); if (r < 0) return log_error_errno(r, "Failed to parse default standard error output setting \"%s\": %m", - optarg); + opts.arg); arg_defaults.std_error = r; break; - case ARG_NO_PAGER: + OPTION_COMMON_NO_PAGER: arg_pager_flags |= PAGER_DISABLE; break; /* Options not shown in --help. */ - case ARG_CRASH_REBOOT: - r = parse_boolean_argument("--crash-reboot", optarg, NULL); + OPTION_LONG_FLAGS(OPTION_OPTIONAL_ARG, "crash-reboot", "BOOL", /* help= */ NULL): + r = parse_boolean_argument("--crash-reboot", opts.arg, NULL); if (r < 0) return r; arg_crash_action = r > 0 ? CRASH_REBOOT : CRASH_FREEZE; break; - case ARG_SERVICE_WATCHDOGS: - r = parse_boolean_argument("--service-watchdogs=", optarg, &arg_service_watchdogs); + OPTION_LONG("service-watchdogs", "BOOL", /* help= */ NULL): + r = parse_boolean_argument("--service-watchdogs=", opts.arg, &arg_service_watchdogs); if (r < 0) return r; break; - case ARG_DESERIALIZE: { - int fd; - FILE *f; - - fd = parse_fd(optarg); + OPTION_LONG("deserialize", "FD", /* help= */ NULL): { + int fd = parse_fd(opts.arg); if (fd < 0) - return log_error_errno(fd, "Failed to parse serialization fd \"%s\": %m", optarg); + return log_error_errno(fd, "Failed to parse serialization fd \"%s\": %m", opts.arg); (void) fd_cloexec(fd, true); - f = fdopen(fd, "r"); + FILE *f = fdopen(fd, "r"); if (!f) return log_error_errno(errno, "Failed to open serialization fd %d: %m", fd); safe_fclose(arg_serialization); arg_serialization = f; - break; } - case ARG_SWITCHED_ROOT: + OPTION_LONG("switched-root", NULL, /* help= */ NULL): arg_switched_root = true; break; - case ARG_MACHINE_ID: - r = id128_from_string_nonzero(optarg, &arg_machine_id); + OPTION_LONG("machine-id", "ID", /* help= */ NULL): + r = id128_from_string_nonzero(opts.arg, &arg_machine_id); if (r < 0) - return log_error_errno(r, "MachineID '%s' is not valid: %m", optarg); + return log_error_errno(r, "MachineID '%s' is not valid: %m", opts.arg); break; - case 'D': + OPTION_SHORT('D', NULL, /* help= */ NULL): log_set_max_level(LOG_DEBUG); break; - case 'b': - case 's': - case 'z': - /* Just to eat away the sysvinit kernel cmdline args that we'll parse in - * parse_proc_cmdline_item() or ignore, without any getopt() error messages. - */ - case '?': - if (getpid_cached() != 1) - return -EINVAL; - else + /* Just to eat away the sysvinit kernel cmdline args that we'll parse in + * parse_proc_cmdline_item() or ignore. */ + OPTION_SHORT('b', NULL, /* help= */ NULL): {} + OPTION_SHORT('s', NULL, /* help= */ NULL): {} + OPTION_SHORT('z', "ARG", /* help= */ NULL): + OPTION_ERROR: + if (getpid_cached() == 1) return 0; - - default: - assert_not_reached(); + return -EINVAL; } - if (optind < argc && getpid_cached() != 1) + if (option_parser_get_n_args(&opts) > 0 && getpid_cached() != 1) /* Hmm, when we aren't run as init system let's complain about excess arguments */ return log_error_errno(SYNTHETIC_ERRNO(EINVAL), "Excess arguments."); @@ -1222,50 +1200,24 @@ static int parse_argv(int argc, char *argv[]) { } static int help(void) { - _cleanup_free_ char *link = NULL; + _cleanup_(table_unrefp) Table *options = NULL; int r; - r = terminal_urlify_man("systemd", "1", &link); + r = option_parser_get_help_table(&options); if (r < 0) - return log_oom(); + return r; - printf("%s [OPTIONS...]\n\n" - "%sStarts and monitors system and user services.%s\n\n" - "This program takes no positional arguments.\n\n" - "%sOptions%s:\n" - " -h --help Show this help\n" - " --version Show version\n" - " --test Determine initial transaction, dump it and exit\n" - " --system Combined with --test: operate in system mode\n" - " --user Combined with --test: operate in user mode\n" - " --dump-configuration-items Dump understood unit configuration items\n" - " --dump-bus-properties Dump exposed bus properties\n" - " --bus-introspect=PATH Write XML introspection data\n" - " --unit=UNIT Set default unit\n" - " --dump-core[=BOOL] Dump core on crash\n" - " --crash-vt=NR Change to specified VT on crash\n" - " --crash-action=ACTION Specify what to do on crash\n" - " --crash-shell[=BOOL] Run shell on crash\n" - " --confirm-spawn[=BOOL] Ask for confirmation when spawning processes\n" - " --show-status[=BOOL] Show status updates on the console during boot\n" - " --log-target=TARGET Set log target (console, journal, kmsg,\n" - " journal-or-kmsg, null)\n" - " --log-level=LEVEL Set log level (debug, info, notice, warning,\n" - " err, crit, alert, emerg)\n" - " --log-color[=BOOL] Highlight important log messages\n" - " --log-location[=BOOL] Include code location in log messages\n" - " --log-time[=BOOL] Prefix log messages with current time\n" - " --default-standard-output= Set default standard output for services\n" - " --default-standard-error= Set default standard error output for services\n" - " --no-pager Do not pipe output into a pager\n" - "\nSee the %s for details.\n", - program_invocation_short_name, - ansi_highlight(), - ansi_normal(), - ansi_underline(), - ansi_normal(), - link); + help_cmdline("[OPTIONS...]"); + help_abstract("Starts and monitors system and user services."); + printf("\nThis program takes no positional arguments.\n"); + + help_section("Options"); + r = table_print_or_warn(options); + if (r < 0) + return r; + + help_man_page_reference("systemd", "1"); return 0; } @@ -2202,8 +2154,8 @@ static int do_reexecute( } /* Try the fallback, if there is any, without any serialization. We pass the original argv[] and - * envp[]. (Well, modulo the ordering changes due to getopt() in argv[], and some cleanups in envp[], - * but let's hope that doesn't matter.) */ + * envp[]. (Well, modulo the ordering changes due to option parsing in argv[], and some cleanups in + * envp[], but let's hope that doesn't matter.) */ arg_serialization = safe_fclose(arg_serialization); fds = fdset_free(fds); From f51393716f2c4aa59b7741bfb5d7d8b1fc5fd601 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Zbigniew=20J=C4=99drzejewski-Szmek?= Date: Thu, 14 May 2026 18:18:24 +0200 Subject: [PATCH 4/8] core: add --crash-vt= for compat with --help Fixes b9e74c399458a1146894ce371e7d85c60658110c. --- src/core/main.c | 5 +++-- 1 file changed, 3 insertions(+), 2 deletions(-) diff --git a/src/core/main.c b/src/core/main.c index d990017ded..9fc2ace1c5 100644 --- a/src/core/main.c +++ b/src/core/main.c @@ -1026,10 +1026,11 @@ static int parse_argv(int argc, char *argv[]) { return r; break; - OPTION_LONG("crash-chvt", "NR", "Change to specified VT on crash"): + OPTION_LONG("crash-chvt", "NR", "Change to specified VT on crash"): {} + OPTION_LONG("crash-vt", "NR", /* help= */ NULL): /* compat option that was previously documented in --help */ r = parse_crash_chvt(opts.arg, &arg_crash_chvt); if (r < 0) - return log_error_errno(r, "Failed to parse crash virtual terminal index: \"%s\": %m", + return log_error_errno(r, "Failed to parse crash virtual terminal index '%s': %m", opts.arg); break; From 63fc9f1696a7d2032280f0bfd8729ecf2fe425e1 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Zbigniew=20J=C4=99drzejewski-Szmek?= Date: Thu, 14 May 2026 18:44:29 +0200 Subject: [PATCH 5/8] core: tweak handling of unknown options 099663ff8c117303af369a4d412dafed0c5614c2 added "support" for -b/-s/-z ARG with a comment of > /* Just to eat away the sysvinit kernel cmdline args without getopt() > * error messages that we'll parse in parse_proc_cmdline_word() or > * ignore. */ And for PID1 those was valid. But when not running as PID1, those options would be parsed as valid but then help() would immediately return -EINVAL: $ build-old/systemd -b; echo $? 1 At the same time, when running as PID1, if we encounter an error, we shouldn't opine about the rest of the command line. So continuing with the loop and the checks after the loop were iffy. Later, cd57038a30aa9447bde3af7111ac8dc517b38bbf made a big refactoring, and the 'break' (i.e. continuation of the loop) was changed to 'return 0', making things even more confusing, since now we'd just silently stop in the middle of the command line if -b/-s/-z were encountered. So be more careful: when running as PID1, stop parsing on error and return from the function. We didn't parse the full command line, so the later checks are not useful. Silently ignore -b/-s/-z. When not running as PID1, explicitly say that -b/-s/-z are not supported, and propogate the error if parsing fails, e.g. with an unknown option. --- src/core/main.c | 14 ++++++++++---- 1 file changed, 10 insertions(+), 4 deletions(-) diff --git a/src/core/main.c b/src/core/main.c index 9fc2ace1c5..3e67186f33 100644 --- a/src/core/main.c +++ b/src/core/main.c @@ -1178,15 +1178,21 @@ static int parse_argv(int argc, char *argv[]) { log_set_max_level(LOG_DEBUG); break; - /* Just to eat away the sysvinit kernel cmdline args that we'll parse in + /* Eat the sysvinit kernel cmdline args that we'll parse in * parse_proc_cmdline_item() or ignore. */ OPTION_SHORT('b', NULL, /* help= */ NULL): {} OPTION_SHORT('s', NULL, /* help= */ NULL): {} OPTION_SHORT('z', "ARG", /* help= */ NULL): + if (getpid_cached() != 1) + return log_error_errno(SYNTHETIC_ERRNO(EINVAL), + "Switch -%c is only supported when running as PID 1.", opts.opt->short_code); + /* In PID1, do nothing and continue with option parsing. */ + break; OPTION_ERROR: - if (getpid_cached() == 1) - return 0; - return -EINVAL; + if (getpid_cached() != 1) + return -EINVAL; + /* In PID1, silently terminate option parsing. */ + return 0; } if (option_parser_get_n_args(&opts) > 0 && getpid_cached() != 1) From 4bfdb3c62f78a34736f6debeade4a849317f5dc7 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Zbigniew=20J=C4=99drzejewski-Szmek?= Date: Thu, 14 May 2026 19:44:22 +0200 Subject: [PATCH 6/8] basic: fold getopt-defs.h into proc-cmdline.c It'd be nice if we could not repeat the information about the option list a second time. But I don't see a nice way to do this, since (by design) with the macro approach, the macros must be intertwined with the parse_argv() code. But that code in turn refers to a bunch of variables, so lifting out the function is not immediately possible. So I think it's best to keep the existing approach where we provide a list of options, without additional context, and skip them using a custom routine. --- TODO.md | 3 ++ src/basic/getopt-defs.h | 77 ---------------------------------------- src/basic/proc-cmdline.c | 60 ++++++++++++++++++++++--------- src/core/main.c | 2 ++ src/shutdown/shutdown.c | 4 ++- 5 files changed, 52 insertions(+), 94 deletions(-) delete mode 100644 src/basic/getopt-defs.h diff --git a/TODO.md b/TODO.md index fe13b0b24d..d9d9e7dd9f 100644 --- a/TODO.md +++ b/TODO.md @@ -3021,6 +3021,9 @@ SPDX-License-Identifier: LGPL-2.1-or-later - whenever we receive fds via SCM_RIGHTS make sure none got dropped due to the reception limit the kernel silently enforces. +- after option+verb introspection is added, add a test to verify that the + list in proc-cmdline.c matches the actual option list in systemd and shutdown. + - write a document explaining how to write correct udev rules. Mention things such as: 1. do not do lists of vid/pid matches, use hwdb for that diff --git a/src/basic/getopt-defs.h b/src/basic/getopt-defs.h deleted file mode 100644 index 9abef6f156..0000000000 --- a/src/basic/getopt-defs.h +++ /dev/null @@ -1,77 +0,0 @@ -/* SPDX-License-Identifier: LGPL-2.1-or-later */ -#pragma once - -#include - -#define SYSTEMD_GETOPT_SHORT_OPTIONS "hDbsz:" - -#define COMMON_GETOPT_ARGS \ - ARG_LOG_LEVEL = 0x100, \ - ARG_LOG_TARGET, \ - ARG_LOG_COLOR, \ - ARG_LOG_LOCATION, \ - ARG_LOG_TIME - -#define SYSTEMD_GETOPT_ARGS \ - ARG_UNIT, \ - ARG_SYSTEM, \ - ARG_USER, \ - ARG_TEST, \ - ARG_NO_PAGER, \ - ARG_VERSION, \ - ARG_DUMP_CONFIGURATION_ITEMS, \ - ARG_DUMP_BUS_PROPERTIES, \ - ARG_BUS_INTROSPECT, \ - ARG_DUMP_CORE, \ - ARG_CRASH_CHVT, \ - ARG_CRASH_SHELL, \ - ARG_CRASH_REBOOT, \ - ARG_CRASH_ACTION, \ - ARG_CONFIRM_SPAWN, \ - ARG_SHOW_STATUS, \ - ARG_DESERIALIZE, \ - ARG_SWITCHED_ROOT, \ - ARG_DEFAULT_STD_OUTPUT, \ - ARG_DEFAULT_STD_ERROR, \ - ARG_MACHINE_ID, \ - ARG_SERVICE_WATCHDOGS - -#define SHUTDOWN_GETOPT_ARGS \ - ARG_EXIT_CODE, \ - ARG_TIMEOUT - -#define COMMON_GETOPT_OPTIONS \ - { "log-level", required_argument, NULL, ARG_LOG_LEVEL }, \ - { "log-target", required_argument, NULL, ARG_LOG_TARGET }, \ - { "log-color", optional_argument, NULL, ARG_LOG_COLOR }, \ - { "log-location", optional_argument, NULL, ARG_LOG_LOCATION }, \ - { "log-time", optional_argument, NULL, ARG_LOG_TIME } - -#define SYSTEMD_GETOPT_OPTIONS \ - { "unit", required_argument, NULL, ARG_UNIT }, \ - { "system", no_argument, NULL, ARG_SYSTEM }, \ - { "user", no_argument, NULL, ARG_USER }, \ - { "test", no_argument, NULL, ARG_TEST }, \ - { "no-pager", no_argument, NULL, ARG_NO_PAGER }, \ - { "help", no_argument, NULL, 'h' }, \ - { "version", no_argument, NULL, ARG_VERSION }, \ - { "dump-configuration-items", no_argument, NULL, ARG_DUMP_CONFIGURATION_ITEMS }, \ - { "dump-bus-properties", no_argument, NULL, ARG_DUMP_BUS_PROPERTIES }, \ - { "bus-introspect", required_argument, NULL, ARG_BUS_INTROSPECT }, \ - { "dump-core", optional_argument, NULL, ARG_DUMP_CORE }, \ - { "crash-chvt", required_argument, NULL, ARG_CRASH_CHVT }, \ - { "crash-shell", optional_argument, NULL, ARG_CRASH_SHELL }, \ - { "crash-reboot", optional_argument, NULL, ARG_CRASH_REBOOT }, \ - { "crash-action", required_argument, NULL, ARG_CRASH_ACTION }, \ - { "confirm-spawn", optional_argument, NULL, ARG_CONFIRM_SPAWN }, \ - { "show-status", optional_argument, NULL, ARG_SHOW_STATUS }, \ - { "deserialize", required_argument, NULL, ARG_DESERIALIZE }, \ - { "switched-root", no_argument, NULL, ARG_SWITCHED_ROOT }, \ - { "default-standard-output", required_argument, NULL, ARG_DEFAULT_STD_OUTPUT, }, \ - { "default-standard-error", required_argument, NULL, ARG_DEFAULT_STD_ERROR, }, \ - { "machine-id", required_argument, NULL, ARG_MACHINE_ID }, \ - { "service-watchdogs", required_argument, NULL, ARG_SERVICE_WATCHDOGS } - -#define SHUTDOWN_GETOPT_OPTIONS \ - { "exit-code", required_argument, NULL, ARG_EXIT_CODE }, \ - { "timeout", required_argument, NULL, ARG_TIMEOUT } diff --git a/src/basic/proc-cmdline.c b/src/basic/proc-cmdline.c index eb61b0bc55..4de3480ff0 100644 --- a/src/basic/proc-cmdline.c +++ b/src/basic/proc-cmdline.c @@ -5,7 +5,6 @@ #include "alloc-util.h" #include "extract-word.h" #include "fileio.h" -#include "getopt-defs.h" #include "initrd-util.h" #include "log.h" #include "parse-util.h" @@ -15,18 +14,50 @@ #include "strv.h" #include "virt.h" +typedef enum ArgType { + no_argument, + required_argument, + optional_argument, +} ArgType; + int proc_cmdline_filter_pid1_args(char **argv, char ***ret) { - enum { - COMMON_GETOPT_ARGS, - SYSTEMD_GETOPT_ARGS, - SHUTDOWN_GETOPT_ARGS, + const struct { + const char *name; + ArgType has_arg; + } options[] = { + { "log-level", required_argument }, + { "log-target", required_argument }, + { "log-color", optional_argument }, + { "log-location", optional_argument }, + { "log-time", optional_argument }, + { "unit", required_argument }, + { "system", no_argument, }, + { "user", no_argument, }, + { "test", no_argument, }, + { "no-pager", no_argument, }, + { "help", no_argument, }, + { "version", no_argument, }, + { "dump-configuration-items", no_argument, }, + { "dump-bus-properties", no_argument, }, + { "bus-introspect", required_argument }, + { "dump-core", optional_argument }, + { "crash-chvt", required_argument }, + { "crash-vt", required_argument }, + { "crash-shell", optional_argument }, + { "crash-reboot", optional_argument }, + { "crash-action", required_argument }, + { "confirm-spawn", optional_argument }, + { "show-status", optional_argument }, + { "deserialize", required_argument }, + { "switched-root", no_argument, }, + { "default-standard-output", required_argument }, + { "default-standard-error", required_argument }, + { "machine-id", required_argument }, + { "service-watchdogs", required_argument }, + { "exit-code", required_argument }, + { "timeout", required_argument }, }; - static const struct option options[] = { - COMMON_GETOPT_OPTIONS, - SYSTEMD_GETOPT_OPTIONS, - SHUTDOWN_GETOPT_OPTIONS, - }; - static const char *short_options = SYSTEMD_GETOPT_SHORT_OPTIONS; + const char *short_options = "hDbsz:"; _cleanup_strv_free_ char **filtered = NULL; int state, r; @@ -108,12 +139,10 @@ int proc_cmdline_filter_pid1_args(char **argv, char ***ret) { } int proc_cmdline(char **ret) { - const char *e; - assert(ret); /* For testing purposes it is sometimes useful to be able to override what we consider /proc/cmdline to be */ - e = secure_getenv("SYSTEMD_PROC_CMDLINE"); + const char *e = secure_getenv("SYSTEMD_PROC_CMDLINE"); if (e) return strdup_to(ret, e); @@ -124,13 +153,12 @@ int proc_cmdline(char **ret) { } static int proc_cmdline_strv_internal(char ***ret, bool filter_pid1_args) { - const char *e; int r; assert(ret); /* For testing purposes it is sometimes useful to be able to override what we consider /proc/cmdline to be */ - e = secure_getenv("SYSTEMD_PROC_CMDLINE"); + const char *e = secure_getenv("SYSTEMD_PROC_CMDLINE"); if (e) return strv_split_full(ret, e, NULL, EXTRACT_UNQUOTE|EXTRACT_RELAX|EXTRACT_RETAIN_ESCAPE); diff --git a/src/core/main.c b/src/core/main.c index 3e67186f33..ce72d4431f 100644 --- a/src/core/main.c +++ b/src/core/main.c @@ -977,6 +977,8 @@ static int parse_argv(int argc, char *argv[]) { int log_level_shift = getpid_cached() == 1 ? LOG_DEBUG - LOG_ERR : 0; OptionParser opts = { argc, argv, .log_level_shift = log_level_shift }; + /* Note: when new options are added here, also add them to the exclusion list in proc-cmdline.c! */ + FOREACH_OPTION(c, &opts) switch (c) { diff --git a/src/shutdown/shutdown.c b/src/shutdown/shutdown.c index 131550c46b..c3c114a667 100644 --- a/src/shutdown/shutdown.c +++ b/src/shutdown/shutdown.c @@ -61,7 +61,9 @@ static int parse_argv(int argc, char *argv[]) { assert(argv); /* The interface is: the verb must stay in argv[1]. Any extra positional arguments - * are warned about and ignored. See 4b5d8d0f22ae61ceb45a25391354ba53b43ee992. */ + * are warned about and ignored. See 4b5d8d0f22ae61ceb45a25391354ba53b43ee992. + * + * Note: when new options are added here, also add them to the exclusion list in proc-cmdline.c! */ OptionParser opts = { argc, argv, OPTION_PARSER_RETURN_POSITIONAL_ARGS }; int r; From 601684ba950424888bde568153addabade8d4627 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Zbigniew=20J=C4=99drzejewski-Szmek?= Date: Fri, 15 May 2026 10:12:00 +0200 Subject: [PATCH 7/8] basic: cache the result of invoked_by_systemd() This function is used in a quite a few places and it calls getenv and does a bit of parsing, so let's try to speed this up. Also, mark it _const_. With the caching, it now always returns the same result, so we can tell the compiler to eliminate subsequent invocations. --- src/basic/argv-util.c | 13 +++++++++---- src/basic/argv-util.h | 2 +- 2 files changed, 10 insertions(+), 5 deletions(-) diff --git a/src/basic/argv-util.c b/src/basic/argv-util.c index c9d610b3a0..0ff2fd9bcb 100644 --- a/src/basic/argv-util.c +++ b/src/basic/argv-util.c @@ -39,27 +39,32 @@ bool invoked_as(char *argv[], const char *token) { } bool invoked_by_systemd(void) { + static int cached = -1; int r; + if (cached >= 0) + return cached; + /* If the process is directly executed by PID1 (e.g. ExecStart= or generator), systemd-importd, * or systemd-homed, then $SYSTEMD_EXEC_PID= is set, and read the command line. */ const char *e = getenv("SYSTEMD_EXEC_PID"); if (!e) - return false; + return (cached = false); if (streq(e, "*")) /* For testing. */ - return true; + return (cached = true); pid_t p; r = parse_pid(e, &p); if (r < 0) { /* We know that systemd sets the variable correctly. Something else must have set it. */ log_debug_errno(r, "Failed to parse \"SYSTEMD_EXEC_PID=%s\", ignoring: %m", e); - return false; + return (cached = false); } - return getpid_cached() == p; + cached = getpid_cached() == p; + return cached; } bool argv_looks_like_help(int argc, char **argv) { diff --git a/src/basic/argv-util.h b/src/basic/argv-util.h index 650b00353d..02901ebc49 100644 --- a/src/basic/argv-util.h +++ b/src/basic/argv-util.h @@ -9,7 +9,7 @@ extern char **saved_argv; void save_argc_argv(int argc, char **argv); bool invoked_as(char *argv[], const char *token); -bool invoked_by_systemd(void); +bool invoked_by_systemd(void) _const_; bool argv_looks_like_help(int argc, char **argv); int rename_process_full(const char *comm, const char *invocation); From 09f107c811a591efa90bd2cfb4982f82c955a349 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Zbigniew=20J=C4=99drzejewski-Szmek?= Date: Fri, 15 May 2026 10:18:28 +0200 Subject: [PATCH 8/8] manager: skip reopening of console and signal reset when running as normal program We want to reopen the console used for logging when running as PID1, but also when running a user manager (c.f. 48a601fe5de8aa0d89ba6dadde168769fa7ce992 and 2a646b1d624e510a79785e1268b55a9c3a441db5). But this can cause problems when the binary is invoked directly, e.g. to print --help. E.g. if we ignore SIGPIPE, we'd remain running briefly after '/usr/lib/systemd/systemd --help | head -n1'. Previusly, the getopt machinery would print to stderr unconditionally. But after the rework of option parsing, which means that we use the log_* functions to repor errors, the test that checks if we print errors to stderr started failing. So let's skip some more of the setup if !invoked_by_systemd(). --- src/core/main.c | 14 ++++++++++---- 1 file changed, 10 insertions(+), 4 deletions(-) diff --git a/src/core/main.c b/src/core/main.c index ce72d4431f..54f510614c 100644 --- a/src/core/main.c +++ b/src/core/main.c @@ -3273,15 +3273,19 @@ int main(int argc, char *argv[]) { /* The efivarfs is now mounted, let's lock down the system token. */ lock_down_efi_variables(); + } else { /* Running as user instance */ arg_runtime_scope = RUNTIME_SCOPE_USER; - log_set_always_reopen_console(true); - log_set_target_and_open(LOG_TARGET_AUTO); /* clear the kernel timestamp, because we are not PID 1 */ kernel_timestamp = DUAL_TIMESTAMP_NULL; + if (invoked_by_systemd()) { + log_set_always_reopen_console(true); + log_set_target_and_open(LOG_TARGET_AUTO); + } + r = mac_init(); if (r < 0) { error_message = "Failed to initialize MAC support"; @@ -3293,9 +3297,11 @@ int main(int argc, char *argv[]) { * transitioning from the initrd to the main systemd or suchlike. */ save_rlimits(&saved_rlimit_nofile, &saved_rlimit_memlock); - /* Reset all signal handlers. */ + /* Reset all signal handlers to clean up after reexec. */ (void) reset_all_signal_handlers(); - (void) ignore_signals(SIGNALS_IGNORE); + + if (getpid_cached() == 1 || invoked_by_systemd()) + (void) ignore_signals(SIGNALS_IGNORE); (void) parse_configuration(&saved_rlimit_nofile, &saved_rlimit_memlock);