From 596031030ee2918a19fc4cc7972caa3bd976c38a Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Zbigniew=20J=C4=99drzejewski-Szmek?= Date: Fri, 24 Apr 2026 13:02:25 +0200 Subject: [PATCH 1/8] shared/format-table: shorten code a bit Define variables at point of initialization so the whole thing is easier to read. --- src/shared/format-table.c | 24 +++++++++--------------- 1 file changed, 9 insertions(+), 15 deletions(-) diff --git a/src/shared/format-table.c b/src/shared/format-table.c index f678a6722c..360fdf596c 100644 --- a/src/shared/format-table.c +++ b/src/shared/format-table.c @@ -2249,11 +2249,7 @@ int _table_sync_column_widths(size_t column, Table *a, ...) { } int table_print_full(Table *t, FILE *f, bool flush) { - size_t n_rows, *minimum_width, *maximum_width, display_columns, *requested_width, - table_minimum_width, table_maximum_width, table_requested_width, table_effective_width, - *width = NULL; _cleanup_free_ size_t *sorted = NULL; - uint64_t *column_weight, weight_sum; int r; assert(t); @@ -2264,7 +2260,7 @@ int table_print_full(Table *t, FILE *f, bool flush) { /* Ensure we have no incomplete rows */ assert(t->n_cells % t->n_columns == 0); - n_rows = t->n_cells / t->n_columns; + size_t n_rows = t->n_cells / t->n_columns; assert(n_rows > 0); /* at least the header row must be complete */ if (t->sort_map) { @@ -2280,17 +2276,14 @@ int table_print_full(Table *t, FILE *f, bool flush) { typesafe_qsort_r(sorted, n_rows, table_data_compare, t); } - if (t->display_map) - display_columns = t->n_display_map; - else - display_columns = t->n_columns; - + size_t display_columns = t->display_map ? t->n_display_map : t->n_columns; assert(display_columns > 0); - minimum_width = newa(size_t, display_columns); - maximum_width = newa(size_t, display_columns); - requested_width = newa(size_t, display_columns); - column_weight = newa0(uint64_t, display_columns); + size_t *minimum_width = newa(size_t, display_columns), + *maximum_width = newa(size_t, display_columns), + *requested_width = newa(size_t, display_columns), + *width = NULL; + uint64_t *column_weight = newa0(uint64_t, display_columns); for (size_t j = 0; j < display_columns; j++) { minimum_width[j] = 1; @@ -2373,10 +2366,11 @@ int table_print_full(Table *t, FILE *f, bool flush) { } /* One space between each column */ + size_t table_requested_width, table_minimum_width, table_maximum_width, table_effective_width; table_requested_width = table_minimum_width = table_maximum_width = display_columns - 1; /* Calculate the total weight for all columns, plus the minimum, maximum and requested width for the table. */ - weight_sum = 0; + uint64_t weight_sum = 0; for (size_t j = 0; j < display_columns; j++) { weight_sum += column_weight[j]; From 42e7fd40e03ca7c2c6d6328d49013694c15c9b6b Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Zbigniew=20J=C4=99drzejewski-Szmek?= Date: Thu, 14 May 2026 11:23:40 +0200 Subject: [PATCH 2/8] shared/verbs: split verbs in two lines when the synopsis is > 25 characters The help tests would not pass because in cases where the verb synopsis is very long, we'd format the table badly if the terminal is fairly narrow. I experimented with a few solutions, but overall, it's hard to achieve very good layout with the automatic formatting. I think the approach in this commit works the best: we end up with an two- or three-line verb synopis, which is similar to what we did manually before. $ COLUMNS=80 build/localectl -h ... Commands: [status] Show current locale settings set-locale LOCALE... Set system locale list-locales Show known locales set-keymap MAP [MAP] Set console and X11 keyboard mappings list-keymaps Show known virtual console keyboard mappings set-x11-keymap LAYOUT [MODEL Set X11 and console keyboard mappings [VARIANT [OPTIONS]]] list-x11-keymap-models Show known X11 keyboard mapping models list-x11-keymap-layouts Show known X11 keyboard mapping layouts list-x11-keymap-variants Show known X11 keyboard mapping variants [LAYOUT] list-x11-keymap-options Show known X11 keyboard mapping options I think that almost nobody actually uses an 80 column terminal, and if they do, they probably don't spend too much time looking at our --help output there. So the goal here is to do something reasonable and robust and get the tests to pass. We can use strjoina here because the strings are fully under our control. --- src/shared/verbs.c | 89 +++++++++++++++++++++++++++++++++++----------- 1 file changed, 69 insertions(+), 20 deletions(-) diff --git a/src/shared/verbs.c b/src/shared/verbs.c index e488af63fe..aab24c0681 100644 --- a/src/shared/verbs.c +++ b/src/shared/verbs.c @@ -7,6 +7,7 @@ #include "log.h" #include "string-util.h" #include "strv.h" +#include "terminal-util.h" #include "verbs.h" #include "virt.h" @@ -160,6 +161,72 @@ int dispatch_verb(int argc, char *argv[], const Verb verbs[], void *userdata) { return _dispatch_verb_with_args(strv_skip(argv, optind), verbs, verbs + n, userdata); } +#define VERB_SYNOPSIS_WIDTH_SANE 25 + +static const char* find_point_to_break(const char *s, size_t max_width) { + /* Locate the first space, preferably after max_width, or the last space otherwise. + * Return the part after the space. */ + + if (strlen(s) <= max_width) + return NULL; + + const char *p = strchr(s + max_width, ' ') ?: strrchr(s, ' '); + return p ? p + 1 : NULL; +} + +static int verb_add_help_one(Table *table, const Verb *verb) { + assert(table); + assert(verb); + + bool is_default = FLAGS_SET(verb->flags, VERB_DEFAULT); + int r; + + /* We indent the option string by two spaces. We could set the minimum cell width and + * right-align for a similar result, but that'd be more work. This is only used for + * display. */ + _cleanup_free_ char *s = strjoin(" ", + is_default ? "[" : "", + verb->verb, + verb->argspec ? " " : "", + strempty(verb->argspec), + is_default ? "]" : ""); + if (!s) + return log_oom(); + + const char *ss = NULL; + if (columns() < VERB_SYNOPSIS_WIDTH_SANE * 4) { + /* If the synopsis is very wide, try to split it up. But do this only if the terminal + * is not very wide. If it _is_ wide, the broken up synopsis would look silly. */ + const char *p = find_point_to_break(s, VERB_SYNOPSIS_WIDTH_SANE), *p2 = NULL; + if (p) { + const char *s1 = strndupa_safe(s, p - s), *s2 = NULL; + + p2 = find_point_to_break(p, VERB_SYNOPSIS_WIDTH_SANE - 4); /* we indent by two spaces more */ + if (p2) + s2 = strndupa_safe(p, p2 - p); + + if (s2) + ss = strjoina(s1, "\n ", s2, "\n ", p2); + else + ss = strjoina(s1, "\n ", p); + } + } + + r = table_add_cell(table, NULL, TABLE_STRING, ss ?: s); + if (r < 0) + return table_log_add_error(r); + + _cleanup_strv_free_ char **t = strv_split(verb->help, /* separators= */ NULL); + if (!t) + return log_oom(); + + r = table_add_many(table, TABLE_STRV_WRAPPED, t); + if (r < 0) + return table_log_add_error(r); + + return 0; +} + int _verbs_get_help_table( const Verb verbs[], const Verb verbs_end[], @@ -192,27 +259,9 @@ int _verbs_get_help_table( /* No help string — we do not show the verb */ continue; - bool is_default = FLAGS_SET(verb->flags, VERB_DEFAULT); - - /* We indent the option string by two spaces. We could set the minimum cell width and - * right-align for a similar result, but that'd be more work. This is only used for - * display. */ - r = table_add_cell_stringf(table, NULL, " %s%s%s%s%s", - is_default ? "[" : "", - verb->verb, - verb->argspec ? " " : "", - strempty(verb->argspec), - is_default ? "]" : ""); + r = verb_add_help_one(table, verb); if (r < 0) - return table_log_add_error(r); - - _cleanup_strv_free_ char **s = strv_split(verb->help, /* separators= */ NULL); - if (!s) - return log_oom(); - - r = table_add_many(table, TABLE_STRV_WRAPPED, s); - if (r < 0) - return table_log_add_error(r); + return r; } table_set_header(table, false); From 92fbc11f6af9ed15ebc0ba8f8e308ac6113db0da Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Zbigniew=20J=C4=99drzejewski-Szmek?= Date: Thu, 14 May 2026 11:51:56 +0200 Subject: [PATCH 3/8] shared/options: introduce OPTION_COMMON_{ENTRY_TOKEN,MAKE_ENTRY_DIRECTORY} --- src/bootctl/bootctl.c | 6 ++---- src/shared/options.h | 9 +++++++++ 2 files changed, 11 insertions(+), 4 deletions(-) diff --git a/src/bootctl/bootctl.c b/src/bootctl/bootctl.c index 881bdfb60f..4bb9742a47 100644 --- a/src/bootctl/bootctl.c +++ b/src/bootctl/bootctl.c @@ -554,15 +554,13 @@ static int parse_argv(int argc, char *argv[], char ***ret_args) { arg_quiet = true; break; - OPTION_LONG("entry-token", "TOKEN", - "Entry token to use for this installation" - " (machine-id, os-id, os-image-id, auto, literal:…)"): + OPTION_COMMON_ENTRY_TOKEN: r = parse_boot_entry_token_type(opts.arg, &arg_entry_token_type, &arg_entry_token); if (r < 0) return r; break; - OPTION_LONG("make-entry-directory", "yes|no|auto", "Create $BOOT/ENTRY-TOKEN/ directory"): {} + OPTION_COMMON_MAKE_ENTRY_DIRECTORY: {} OPTION_LONG("make-machine-id-directory", "BOOL", /* help= */ NULL): /* Compatibility alias */ if (streq(opts.arg, "auto")) /* retained for backwards compatibility */ arg_make_entry_directory = -1; /* yes if machine-id is permanent */ diff --git a/src/shared/options.h b/src/shared/options.h index d60c9afe81..5ba276eaee 100644 --- a/src/shared/options.h +++ b/src/shared/options.h @@ -137,6 +137,15 @@ typedef struct Option { OPTION_SHORT('j', NULL, \ "Equivalent to --json=pretty (on TTY) or --json=short (otherwise)") +#define OPTION_COMMON_ENTRY_TOKEN \ + OPTION_LONG("entry-token", "TOKEN", \ + "Entry token to use for this installation " \ + "(machine-id, os-id, os-image-id, auto, literal:…)") + +#define OPTION_COMMON_MAKE_ENTRY_DIRECTORY \ + OPTION_LONG("make-entry-directory", \ + "BOOL|auto", "Create $BOOT/ENTRY-TOKEN/ directory") + #define OPTION_COMMON_PRIVATE_KEY(purpose) \ OPTION_LONG("private-key", "PATH|URI", purpose) From 71c3123b17e54841bd5e37f51b10212375f8b6c3 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Zbigniew=20J=C4=99drzejewski-Szmek?= Date: Thu, 16 Apr 2026 10:25:49 +0200 Subject: [PATCH 4/8] kernel-install: convert to the new option and verb parsers The verb synopses are very long because of the many parameters. Previously were shown without help and occupied all available columns. With the autogenerated help format, this doesn't work great. So the verbs and options tables are not synced, so that help for options can use more columns. I think in this case this is better than the alternatives. Co-developed-by: Claude Opus 4.6 --- src/kernel-install/kernel-install.c | 251 ++++++++++++---------------- 1 file changed, 103 insertions(+), 148 deletions(-) diff --git a/src/kernel-install/kernel-install.c b/src/kernel-install/kernel-install.c index 331923e9e5..0bd4e84da0 100644 --- a/src/kernel-install/kernel-install.c +++ b/src/kernel-install/kernel-install.c @@ -1,10 +1,10 @@ /* SPDX-License-Identifier: LGPL-2.1-or-later */ -#include #include #include #include +#include "ansi-color.h" #include "argv-util.h" #include "boot-entry.h" #include "bootspec.h" @@ -22,6 +22,7 @@ #include "find-esp.h" #include "format-table.h" #include "fs-util.h" +#include "help-util.h" #include "id128-util.h" #include "image-policy.h" #include "kernel-config.h" @@ -29,9 +30,9 @@ #include "loop-util.h" #include "main-func.h" #include "mount-util.h" +#include "options.h" #include "parse-argument.h" #include "path-util.h" -#include "pretty-print.h" #include "recurse-dir.h" #include "rm-rf.h" #include "stat-util.h" @@ -1190,6 +1191,8 @@ static int do_add( return context_execute(c); } +VERB(verb_add, "add", "[[[KERNEL-VERSION] KERNEL-IMAGE] [INITRD ...]]", 1, VERB_ANY, 0, + "Add a kernel and initrd images to the boot partition"); static int verb_add(int argc, char *argv[], uintptr_t _data, void *userdata) { const char *version, *kernel; char **initrds; @@ -1219,6 +1222,8 @@ static int verb_add(int argc, char *argv[], uintptr_t _data, void *userdata) { return do_add(&c, version, kernel, initrds); } +VERB_NOARG(verb_add_all, "add-all", + "Add all kernels found in /usr/lib/modules/"); static int verb_add_all(int argc, char *argv[], uintptr_t _data, void *userdata) { _cleanup_close_ int fd = -EBADF; size_t n = 0; @@ -1299,16 +1304,18 @@ static int verb_add_all(int argc, char *argv[], uintptr_t _data, void *userdata) return ret; } -static int run_as_installkernel(int argc, char *argv[]) { +static int run_as_installkernel(char **args) { /* kernel's install.sh invokes us as * /sbin/installkernel * We ignore the last two arguments. */ - if (optind + 2 > argc) + if (strv_length(args) < 2) return log_error_errno(SYNTHETIC_ERRNO(EINVAL), "'installkernel' command requires at least two arguments."); - return verb_add(3, STRV_MAKE("add", argv[optind], argv[optind+1]), /* data= */ 0, /* userdata= */ NULL); + return verb_add(3, STRV_MAKE("add", args[0], args[1]), /* data= */ 0, /* userdata= */ NULL); } +VERB(verb_remove, "remove", "KERNEL-VERSION", 2, VERB_ANY, 0, + "Remove a kernel from the boot partition"); static int verb_remove(int argc, char *argv[], uintptr_t _data, void *userdata) { int r; @@ -1345,6 +1352,8 @@ static int verb_remove(int argc, char *argv[], uintptr_t _data, void *userdata) return context_execute(&c); } +VERB(verb_inspect, "inspect", "[[[KERNEL-VERSION] KERNEL-IMAGE] [INITRD ...]]", 1, VERB_ANY, VERB_DEFAULT, + "Print details about the installation"); static int verb_inspect(int argc, char *argv[], uintptr_t _data, void *userdata) { _cleanup_(table_unrefp) Table *t = NULL; _cleanup_free_ char *vmlinuz = NULL; @@ -1458,6 +1467,8 @@ static int verb_inspect(int argc, char *argv[], uintptr_t _data, void *userdata) return table_print_with_pager(t, arg_json_format_flags, arg_pager_flags, /* show_header= */ false); } +VERB_NOARG(verb_list, "list", + "List installed kernels"); static int verb_list(int argc, char *argv[], uintptr_t _data, void *userdata) { _cleanup_close_ int fd = -EBADF; int r; @@ -1526,128 +1537,87 @@ static int verb_list(int argc, char *argv[], uintptr_t _data, void *userdata) { } static int help(void) { - _cleanup_free_ char *link = NULL; + _cleanup_(table_unrefp) Table *options = NULL, *verbs = NULL; int r; - r = terminal_urlify_man("kernel-install", "8", &link); + r = verbs_get_help_table(&verbs); if (r < 0) - return log_oom(); + return r; - printf("%1$s [OPTIONS...] COMMAND ...\n\n" - "%5$sAdd and remove kernel and initrd images to and from the boot partition.%6$s\n" - "\n%3$sUsage:%4$s\n" - " kernel-install [OPTIONS...] add [[[KERNEL-VERSION] KERNEL-IMAGE] [INITRD ...]]\n" - " kernel-install [OPTIONS...] add-all\n" - " kernel-install [OPTIONS...] remove KERNEL-VERSION\n" - " kernel-install [OPTIONS...] inspect [[[KERNEL-VERSION] KERNEL-IMAGE]\n" - " [INITRD ...]]\n" - " kernel-install [OPTIONS...] list\n" - "\n%3$sOptions:%4$s\n" - " -h --help Show this help\n" - " --version Show package version\n" - " -v --verbose Increase verbosity\n" - " --esp-path=PATH Path to the EFI System Partition (ESP)\n" - " --boot-path=PATH Path to the $BOOT partition\n" - " --make-entry-directory=yes|no|auto\n" - " Create $BOOT/ENTRY-TOKEN/ directory\n" - " --entry-type=type1|type2|all\n" - " Operate only on the specified bootloader\n" - " entry type\n" - " --entry-token=machine-id|os-id|os-image-id|auto|literal:…\n" - " Entry token to be used for this installation\n" - " --no-pager Do not pipe inspect output into a pager\n" - " --json=pretty|short|off Generate JSON output\n" - " --no-legend Do not show the headers and footers\n" - " --root=PATH Operate on an alternate filesystem root\n" - " --image=PATH Operate on disk image as filesystem root\n" - " --image-policy=POLICY Specify disk image dissection policy\n" - "\n" + r = option_parser_get_help_table(&options); + if (r < 0) + return r; + + /* Note: column widths are not synced, because the verbs table is very wide. */ + + help_cmdline("[OPTIONS…] COMMAND …"); + help_abstract("Add and remove kernel and initrd images to and from the boot partition."); + + help_section("Commands"); + r = table_print_or_warn(verbs); + if (r < 0) + return r; + + help_section("Options"); + r = table_print_or_warn(options); + if (r < 0) + return r; + + printf("\n" "This program may also be invoked as 'installkernel':\n" - " installkernel [OPTIONS...] VERSION VMLINUZ [MAP] [INSTALLATION-DIR]\n" - "(The optional arguments are passed by kernel build system, but ignored.)\n" - "\n" - "See the %2$s for details.\n", - program_invocation_short_name, - link, - ansi_underline(), - ansi_normal(), - ansi_highlight(), - ansi_normal()); + " installkernel [OPTIONS...] VERSION VMLINUZ [MAP] [INSTALLATION-DIR]\n" + "(The optional arguments are passed by kernel build system, but ignored.)\n"); + + help_man_page_reference("kernel-install", "8"); return 0; } -static int parse_argv(int argc, char *argv[]) { - enum { - ARG_VERSION = 0x100, - ARG_NO_LEGEND, - ARG_ESP_PATH, - ARG_BOOT_PATH, - ARG_MAKE_ENTRY_DIRECTORY, - ARG_ENTRY_TOKEN, - ARG_NO_PAGER, - ARG_JSON, - ARG_ROOT, - ARG_IMAGE, - ARG_IMAGE_POLICY, - ARG_BOOT_ENTRY_TYPE, - }; - static const struct option options[] = { - { "help", no_argument, NULL, 'h' }, - { "version", no_argument, NULL, ARG_VERSION }, - { "verbose", no_argument, NULL, 'v' }, - { "esp-path", required_argument, NULL, ARG_ESP_PATH }, - { "boot-path", required_argument, NULL, ARG_BOOT_PATH }, - { "make-entry-directory", required_argument, NULL, ARG_MAKE_ENTRY_DIRECTORY }, - { "entry-token", required_argument, NULL, ARG_ENTRY_TOKEN }, - { "no-pager", no_argument, NULL, ARG_NO_PAGER }, - { "json", required_argument, NULL, ARG_JSON }, - { "root", required_argument, NULL, ARG_ROOT }, - { "image", required_argument, NULL, ARG_IMAGE }, - { "image-policy", required_argument, NULL, ARG_IMAGE_POLICY }, - { "no-legend", no_argument, NULL, ARG_NO_LEGEND }, - { "entry-type", required_argument, NULL, ARG_BOOT_ENTRY_TYPE }, - {} - }; - int t, r; +VERB_COMMON_HELP(help); +static int parse_argv(int argc, char *argv[], char ***remaining_args) { assert(argc >= 0); assert(argv); + assert(remaining_args); - while ((t = getopt_long(argc, argv, "hv", options, NULL)) >= 0) - switch (t) { - case 'h': + OptionParser opts = { argc, argv }; + int r; + + FOREACH_OPTION_OR_RETURN(c, &opts) + switch (c) { + + OPTION_COMMON_HELP: return help(); - case ARG_VERSION: + OPTION_COMMON_VERSION: return version(); - case ARG_NO_LEGEND: + OPTION_COMMON_NO_LEGEND: arg_legend = false; break; - case 'v': + OPTION('v', "verbose", NULL, "Increase verbosity"): log_set_max_level(LOG_DEBUG); arg_verbose = true; break; - case ARG_ESP_PATH: - r = parse_path_argument(optarg, /* suppress_root= */ false, &arg_esp_path); + OPTION_LONG("esp-path", "PATH", "Path to the EFI System Partition (ESP)"): + r = parse_path_argument(opts.arg, /* suppress_root= */ false, &arg_esp_path); if (r < 0) return log_oom(); break; - case ARG_BOOT_PATH: - r = parse_path_argument(optarg, /* suppress_root= */ false, &arg_xbootldr_path); + OPTION_LONG("boot-path", "PATH", "Path to the $BOOT partition"): + r = parse_path_argument(opts.arg, /* suppress_root= */ false, &arg_xbootldr_path); if (r < 0) return log_oom(); break; - case ARG_MAKE_ENTRY_DIRECTORY: - if (streq(optarg, "auto")) + OPTION_COMMON_MAKE_ENTRY_DIRECTORY: + if (streq(opts.arg, "auto")) arg_make_entry_directory = -1; else { - r = parse_boolean_argument("--make-entry-directory=", optarg, NULL); + r = parse_boolean_argument("--make-entry-directory=", opts.arg, NULL); if (r < 0) return r; @@ -1655,85 +1625,70 @@ static int parse_argv(int argc, char *argv[]) { } break; - case ARG_ENTRY_TOKEN: - r = parse_boot_entry_token_type(optarg, &arg_entry_token_type, &arg_entry_token); + OPTION_COMMON_ENTRY_TOKEN: + r = parse_boot_entry_token_type(opts.arg, &arg_entry_token_type, &arg_entry_token); if (r < 0) return r; break; - case ARG_NO_PAGER: - arg_pager_flags |= PAGER_DISABLE; - break; - - case ARG_JSON: - r = parse_json_argument(optarg, &arg_json_format_flags); - if (r <= 0) - return r; - break; - - case ARG_ROOT: - r = parse_path_argument(optarg, /* suppress_root= */ false, &arg_root); - if (r < 0) - return r; - break; - - case ARG_IMAGE: - r = parse_path_argument(optarg, /* suppress_root= */ false, &arg_image); - if (r < 0) - return r; - break; - - case ARG_IMAGE_POLICY: - r = parse_image_policy_argument(optarg, &arg_image_policy); - if (r < 0) - return r; - break; - - case ARG_BOOT_ENTRY_TYPE: { - if (isempty(optarg) || streq(optarg, "all")) { + OPTION_LONG("entry-type", "TYPE", + "Operate only on the specified bootloader entry type (type1, type2, all)"): { + if (isempty(opts.arg) || streq(opts.arg, "all")) { arg_boot_entry_type = _BOOT_ENTRY_TYPE_INVALID; break; } - BootEntryType e = boot_entry_type_from_string(optarg); + BootEntryType e = boot_entry_type_from_string(opts.arg); if (e < 0) - return log_error_errno(e, "Invalid entry type: %s", optarg); + return log_error_errno(e, "Invalid entry type: %s", opts.arg); arg_boot_entry_type = e; break; } - case '?': - return -EINVAL; + OPTION_COMMON_NO_PAGER: + arg_pager_flags |= PAGER_DISABLE; + break; - default: - assert_not_reached(); + OPTION_COMMON_JSON: + r = parse_json_argument(opts.arg, &arg_json_format_flags); + if (r <= 0) + return r; + break; + + OPTION_LONG("root", "PATH", "Operate on an alternate filesystem root"): + r = parse_path_argument(opts.arg, /* suppress_root= */ false, &arg_root); + if (r < 0) + return r; + break; + + OPTION_LONG("image", "PATH", "Operate on disk image as filesystem root"): + r = parse_path_argument(opts.arg, /* suppress_root= */ false, &arg_image); + if (r < 0) + return r; + break; + + OPTION_LONG("image-policy", "POLICY", "Specify disk image dissection policy"): + r = parse_image_policy_argument(opts.arg, &arg_image_policy); + if (r < 0) + return r; + break; } if (arg_image && arg_root) - return log_error_errno(SYNTHETIC_ERRNO(EINVAL), "Please specify either --root= or --image=, the combination of both is not supported."); + return log_error_errno(SYNTHETIC_ERRNO(EINVAL), + "Please specify either --root= or --image=, the combination of both is not supported."); + *remaining_args = option_parser_get_args(&opts); return 1; } -static int kernel_install_main(int argc, char *argv[]) { - static const Verb verbs[] = { - { "add", 1, VERB_ANY, 0, verb_add }, - { "add-all", 1, 1, 0, verb_add_all }, - { "remove", 2, VERB_ANY, 0, verb_remove }, - { "inspect", 1, VERB_ANY, VERB_DEFAULT, verb_inspect }, - { "list", 1, 1, 0, verb_list }, - {} - }; - - return dispatch_verb(argc, argv, verbs, /* userdata= */ NULL); -} - static int run(int argc, char* argv[]) { int r; log_setup(); - r = parse_argv(argc, argv); + char **args = NULL; + r = parse_argv(argc, argv, &args); if (r <= 0) return r; @@ -1762,9 +1717,9 @@ static int run(int argc, char* argv[]) { } if (invoked_as(argv, "installkernel")) - return run_as_installkernel(argc, argv); + return run_as_installkernel(args); - return kernel_install_main(argc, argv); + return dispatch_verb_with_args(args, /* userdata= */ NULL); } DEFINE_MAIN_FUNCTION_WITH_POSITIVE_FAILURE(run); From b333fcf9dae48df14480775c009e7c13de6ca5b8 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Zbigniew=20J=C4=99drzejewski-Szmek?= Date: Thu, 16 Apr 2026 11:38:06 +0200 Subject: [PATCH 5/8] localectl: convert to the new option and verb parsers MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit The verb synopses are long, so they got broken up: =============================================================================== > localectl [OPTIONS…] COMMAND … Query or change system locale and keyboard settings. Commands: [status] Show current locale settings set-locale LOCALE... Set system locale list-locales Show known locales set-keymap MAP [MAP] Set console and X11 keyboard mappings list-keymaps Show known virtual console keyboard mappings set-x11-keymap LAYOUT [MODEL Set X11 and console keyboard mappings [VARIANT [OPTIONS]]] list-x11-keymap-models Show known X11 keyboard mapping models list-x11-keymap-layouts Show known X11 keyboard mapping layouts list-x11-keymap-variants Show known X11 keyboard mapping variants [LAYOUT] list-x11-keymap-options Show known X11 keyboard mapping options Options: -h --help Show this help --version Show package version -l --full Do not ellipsize output --no-pager Do not start a pager --no-ask-password Do not prompt for password -H --host=[USER@]HOST Operate on remote host -M --machine=CONTAINER Operate on local container --no-convert Don't convert keyboard mappings See the localectl(1) man page for details. =============================================================================== But I think this is OK. Everything is readable. On a more normal terminal, everything fits nicely. Co-developed-by: Claude Opus 4.6 --- src/locale/localectl.c | 170 +++++++++++++++++------------------------ 1 file changed, 68 insertions(+), 102 deletions(-) diff --git a/src/locale/localectl.c b/src/locale/localectl.c index e80cd96c86..e20d5da9a0 100644 --- a/src/locale/localectl.c +++ b/src/locale/localectl.c @@ -1,9 +1,8 @@ /* SPDX-License-Identifier: LGPL-2.1-or-later */ -#include - #include "sd-bus.h" +#include "ansi-color.h" #include "alloc-util.h" #include "build.h" #include "bus-error.h" @@ -13,15 +12,16 @@ #include "fd-util.h" #include "fileio.h" #include "format-table.h" +#include "help-util.h" #include "kbd-util.h" #include "locale-setup.h" #include "main-func.h" #include "memory-util.h" +#include "options.h" #include "pager.h" #include "parse-argument.h" #include "path-util.h" #include "polkit-agent.h" -#include "pretty-print.h" #include "runtime-scope.h" #include "string-util.h" #include "strv.h" @@ -147,6 +147,8 @@ static int print_status_info(StatusInfo *i) { return table_print_or_warn(table); } +VERB(verb_show_status, "status", NULL, VERB_ANY, 1, VERB_DEFAULT, + "Show current locale settings"); static int verb_show_status(int argc, char *argv[], uintptr_t _data, void *userdata) { _cleanup_(status_info_clear) StatusInfo info = {}; static const struct bus_properties_map map[] = { @@ -179,6 +181,8 @@ static int verb_show_status(int argc, char *argv[], uintptr_t _data, void *userd return print_status_info(&info); } +VERB(verb_set_locale, "set-locale", "LOCALE...", 2, VERB_ANY, 0, + "Set system locale"); static int verb_set_locale(int argc, char *argv[], uintptr_t _data, void *userdata) { _cleanup_(sd_bus_message_unrefp) sd_bus_message *m = NULL; _cleanup_(sd_bus_error_free) sd_bus_error error = SD_BUS_ERROR_NULL; @@ -207,6 +211,8 @@ static int verb_set_locale(int argc, char *argv[], uintptr_t _data, void *userda return 0; } +VERB_NOARG(verb_list_locales, "list-locales", + "Show known locales"); static int verb_list_locales(int argc, char *argv[], uintptr_t _data, void *userdata) { _cleanup_strv_free_ char **l = NULL; int r; @@ -221,6 +227,8 @@ static int verb_list_locales(int argc, char *argv[], uintptr_t _data, void *user return 0; } +VERB(verb_set_vconsole_keymap, "set-keymap", "MAP [MAP]", 2, 3, 0, + "Set console and X11 keyboard mappings"); static int verb_set_vconsole_keymap(int argc, char *argv[], uintptr_t _data, void *userdata) { _cleanup_(sd_bus_error_free) sd_bus_error error = SD_BUS_ERROR_NULL; const char *map, *toggle_map; @@ -245,6 +253,8 @@ static int verb_set_vconsole_keymap(int argc, char *argv[], uintptr_t _data, voi return 0; } +VERB_NOARG(verb_list_vconsole_keymaps, "list-keymaps", + "Show known virtual console keyboard mappings"); static int verb_list_vconsole_keymaps(int argc, char *argv[], uintptr_t _data, void *userdata) { _cleanup_strv_free_ char **l = NULL; int r; @@ -260,6 +270,8 @@ static int verb_list_vconsole_keymaps(int argc, char *argv[], uintptr_t _data, v return 0; } +VERB(verb_set_x11_keymap, "set-x11-keymap", "LAYOUT [MODEL [VARIANT [OPTIONS]]]", 2, 5, 0, + "Set X11 and console keyboard mappings"); static int verb_set_x11_keymap(int argc, char *argv[], uintptr_t _data, void *userdata) { _cleanup_(sd_bus_error_free) sd_bus_error error = SD_BUS_ERROR_NULL; const char *layout, *model, *variant, *options; @@ -295,6 +307,14 @@ static const char* xkb_directory(void) { return cached; } +VERB_NOARG(verb_list_x11_keymaps, "list-x11-keymap-models", + "Show known X11 keyboard mapping models"); +VERB_NOARG(verb_list_x11_keymaps, "list-x11-keymap-layouts", + "Show known X11 keyboard mapping layouts"); +VERB(verb_list_x11_keymaps, "list-x11-keymap-variants", "[LAYOUT]", VERB_ANY, 2, 0, + "Show known X11 keyboard mapping variants"); +VERB_NOARG(verb_list_x11_keymaps, "list-x11-keymap-options", + "Show known X11 keyboard mapping options"); static int verb_list_x11_keymaps(int argc, char *argv[], uintptr_t _data, void *userdata) { _cleanup_fclose_ FILE *f = NULL; _cleanup_strv_free_ char **list = NULL; @@ -402,143 +422,88 @@ static int verb_list_x11_keymaps(int argc, char *argv[], uintptr_t _data, void * } static int help(void) { - _cleanup_free_ char *link = NULL; + _cleanup_(table_unrefp) Table *options = NULL, *verbs = NULL; int r; - r = terminal_urlify_man("localectl", "1", &link); + r = verbs_get_help_table(&verbs); if (r < 0) - return log_oom(); + return r; - printf("%s [OPTIONS...] COMMAND ...\n\n" - "%sQuery or change system locale and keyboard settings.%s\n" - "\nCommands:\n" - " status Show current locale settings\n" - " set-locale LOCALE... Set system locale\n" - " list-locales Show known locales\n" - " set-keymap MAP [MAP] Set console and X11 keyboard mappings\n" - " list-keymaps Show known virtual console keyboard mappings\n" - " set-x11-keymap LAYOUT [MODEL [VARIANT [OPTIONS]]]\n" - " Set X11 and console keyboard mappings\n" - " list-x11-keymap-models Show known X11 keyboard mapping models\n" - " list-x11-keymap-layouts Show known X11 keyboard mapping layouts\n" - " list-x11-keymap-variants [LAYOUT]\n" - " Show known X11 keyboard mapping variants\n" - " list-x11-keymap-options Show known X11 keyboard mapping options\n" - "\nOptions:\n" - " -h --help Show this help\n" - " --version Show package version\n" - " -l --full Do not ellipsize output\n" - " --no-pager Do not pipe output into a pager\n" - " --no-ask-password Do not prompt for password\n" - " -H --host=[USER@]HOST Operate on remote host\n" - " -M --machine=CONTAINER Operate on local container\n" - " --no-convert Don't convert keyboard mappings\n" - "\nSee the %s for details.\n", - program_invocation_short_name, - ansi_highlight(), - ansi_normal(), - link); + r = option_parser_get_help_table(&options); + if (r < 0) + return r; + + (void) table_sync_column_widths(0, verbs, options); + + help_cmdline("[OPTIONS…] COMMAND …"); + help_abstract("Query or change system locale and keyboard settings."); + + help_section("Commands"); + r = table_print_or_warn(verbs); + if (r < 0) + return r; + + help_section("Options"); + r = table_print_or_warn(options); + if (r < 0) + return r; + + help_man_page_reference("localectl", "1"); return 0; } -static int verb_help(int argc, char *argv[], uintptr_t _data, void *userdata) { - return help(); -} - -static int parse_argv(int argc, char *argv[]) { - - enum { - ARG_VERSION = 0x100, - ARG_NO_PAGER, - ARG_NO_CONVERT, - ARG_NO_ASK_PASSWORD - }; - - static const struct option options[] = { - { "help", no_argument, NULL, 'h' }, - { "version", no_argument, NULL, ARG_VERSION }, - { "full", no_argument, NULL, 'l' }, - { "no-pager", no_argument, NULL, ARG_NO_PAGER }, - { "host", required_argument, NULL, 'H' }, - { "machine", required_argument, NULL, 'M' }, - { "no-ask-password", no_argument, NULL, ARG_NO_ASK_PASSWORD }, - { "no-convert", no_argument, NULL, ARG_NO_CONVERT }, - {} - }; - - int r, c; +VERB_COMMON_HELP_HIDDEN(help); +static int parse_argv(int argc, char *argv[], char ***remaining_args) { assert(argc >= 0); assert(argv); + assert(remaining_args); - while ((c = getopt_long(argc, argv, "hlH:M:", options, NULL)) >= 0) + OptionParser opts = { argc, argv }; + int r; + FOREACH_OPTION_OR_RETURN(c, &opts) switch (c) { - case 'h': + OPTION_COMMON_HELP: return help(); - case ARG_VERSION: + OPTION_COMMON_VERSION: return version(); - case 'l': + OPTION('l', "full", NULL, "Do not ellipsize output"): arg_full = true; break; - case ARG_NO_CONVERT: - arg_convert = false; - break; - - case ARG_NO_PAGER: + OPTION_COMMON_NO_PAGER: arg_pager_flags |= PAGER_DISABLE; break; - case ARG_NO_ASK_PASSWORD: + OPTION_COMMON_NO_ASK_PASSWORD: arg_ask_password = false; break; - case 'H': + OPTION_COMMON_HOST: arg_transport = BUS_TRANSPORT_REMOTE; - arg_host = optarg; + arg_host = opts.arg; break; - case 'M': - r = parse_machine_argument(optarg, &arg_host, &arg_transport); + OPTION_COMMON_MACHINE: + r = parse_machine_argument(opts.arg, &arg_host, &arg_transport); if (r < 0) return r; break; - case '?': - return -EINVAL; - - default: - assert_not_reached(); + OPTION_LONG("no-convert", NULL, "Don't convert keyboard mappings"): + arg_convert = false; + break; } + *remaining_args = option_parser_get_args(&opts); return 1; } -static int localectl_main(sd_bus *bus, int argc, char *argv[]) { - - static const Verb verbs[] = { - { "status", VERB_ANY, 1, VERB_DEFAULT, verb_show_status }, - { "set-locale", 2, VERB_ANY, 0, verb_set_locale }, - { "list-locales", VERB_ANY, 1, 0, verb_list_locales }, - { "set-keymap", 2, 3, 0, verb_set_vconsole_keymap }, - { "list-keymaps", VERB_ANY, 1, 0, verb_list_vconsole_keymaps }, - { "set-x11-keymap", 2, 5, 0, verb_set_x11_keymap }, - { "list-x11-keymap-models", VERB_ANY, 1, 0, verb_list_x11_keymaps }, - { "list-x11-keymap-layouts", VERB_ANY, 1, 0, verb_list_x11_keymaps }, - { "list-x11-keymap-variants", VERB_ANY, 2, 0, verb_list_x11_keymaps }, - { "list-x11-keymap-options", VERB_ANY, 1, 0, verb_list_x11_keymaps }, - { "help", VERB_ANY, VERB_ANY, 0, verb_help }, /* Not documented, but supported since it has been created. */ - {} - }; - - return dispatch_verb(argc, argv, verbs, bus); -} - static int run(int argc, char *argv[]) { _cleanup_(sd_bus_flush_close_unrefp) sd_bus *bus = NULL; int r; @@ -546,7 +511,8 @@ static int run(int argc, char *argv[]) { setlocale(LC_ALL, ""); log_setup(); - r = parse_argv(argc, argv); + char **args = NULL; + r = parse_argv(argc, argv, &args); if (r <= 0) return r; @@ -554,7 +520,7 @@ static int run(int argc, char *argv[]) { if (r < 0) return bus_log_connect_error(r, arg_transport, RUNTIME_SCOPE_SYSTEM); - return localectl_main(bus, argc, argv); + return dispatch_verb_with_args(args, bus); } DEFINE_MAIN_FUNCTION(run); From d0f67f257d62c51dcc2d48f15e3793ad16856b3b Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Zbigniew=20J=C4=99drzejewski-Szmek?= Date: Tue, 14 Apr 2026 16:42:41 +0200 Subject: [PATCH 6/8] busctl: reorder option cases to match --help output Co-developed-by: Claude Opus 4.6 --- src/busctl/busctl.c | 70 ++++++++++++++++++++++----------------------- 1 file changed, 35 insertions(+), 35 deletions(-) diff --git a/src/busctl/busctl.c b/src/busctl/busctl.c index a895c3fe91..1e6fb64e96 100644 --- a/src/busctl/busctl.c +++ b/src/busctl/busctl.c @@ -2220,12 +2220,34 @@ static int parse_argv(int argc, char *argv[]) { arg_full = true; break; + case ARG_SYSTEM: + arg_runtime_scope = RUNTIME_SCOPE_SYSTEM; + break; + case ARG_USER: arg_runtime_scope = RUNTIME_SCOPE_USER; break; - case ARG_SYSTEM: - arg_runtime_scope = RUNTIME_SCOPE_SYSTEM; + case 'H': + arg_transport = BUS_TRANSPORT_REMOTE; + arg_host = optarg; + break; + + case 'M': + r = parse_machine_argument(optarg, &arg_host, &arg_transport); + if (r < 0) + return r; + break; + + case 'C': + r = capsule_name_is_valid(optarg); + if (r < 0) + return log_error_errno(r, "Unable to validate capsule name '%s': %m", optarg); + if (r == 0) + return log_error_errno(SYNTHETIC_ERRNO(EINVAL), "Invalid capsule name: %s", optarg); + + arg_host = optarg; + arg_transport = BUS_TRANSPORT_CAPSULE; break; case ARG_ADDRESS: @@ -2272,28 +2294,6 @@ static int parse_argv(int argc, char *argv[]) { arg_list = true; break; - case 'H': - arg_transport = BUS_TRANSPORT_REMOTE; - arg_host = optarg; - break; - - case 'M': - r = parse_machine_argument(optarg, &arg_host, &arg_transport); - if (r < 0) - return r; - break; - - case 'C': - r = capsule_name_is_valid(optarg); - if (r < 0) - return log_error_errno(r, "Unable to validate capsule name '%s': %m", optarg); - if (r == 0) - return log_error_errno(SYNTHETIC_ERRNO(EINVAL), "Invalid capsule name: %s", optarg); - - arg_host = optarg; - arg_transport = BUS_TRANSPORT_CAPSULE; - break; - case 'q': arg_quiet = true; break; @@ -2302,6 +2302,17 @@ static int parse_argv(int argc, char *argv[]) { arg_verbose = true; break; + case ARG_JSON: + r = parse_json_argument(optarg, &arg_json_format_flags); + if (r <= 0) + return r; + + break; + + case 'j': + arg_json_format_flags = SD_JSON_FORMAT_PRETTY_AUTO|SD_JSON_FORMAT_COLOR_AUTO; + break; + case ARG_XML_INTERFACE: arg_xml_interface = true; break; @@ -2349,17 +2360,6 @@ static int parse_argv(int argc, char *argv[]) { return r; break; - case 'j': - arg_json_format_flags = SD_JSON_FORMAT_PRETTY_AUTO|SD_JSON_FORMAT_COLOR_AUTO; - break; - - case ARG_JSON: - r = parse_json_argument(optarg, &arg_json_format_flags); - if (r <= 0) - return r; - - break; - case ARG_DESTINATION: arg_destination = optarg; break; From d2aafc33eaeb3756ca2167a041d2334989bbaf58 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Zbigniew=20J=C4=99drzejewski-Szmek?= Date: Tue, 14 Apr 2026 12:56:01 +0200 Subject: [PATCH 7/8] busctl: convert to the new option and verb parsers The conversion doesn't work great, because some of the verbs take many arguments and the first column is extermely wide. So similarly to kernel-install, I dropped the sync of column widths. This allows the help for options to use most of the available space. -C/--capsule is now documented, fixup for 00431b2b66cb59540deda4ea018170a289673585. Verb functions are renamed to match verb names. The missing first param is added to the synopsis of "wait". It now matches the man page. Co-developed-by: Claude Opus 4.6 --- src/busctl/busctl.c | 347 ++++++++++++++++---------------------------- 1 file changed, 122 insertions(+), 225 deletions(-) diff --git a/src/busctl/busctl.c b/src/busctl/busctl.c index 1e6fb64e96..79ee1c55e8 100644 --- a/src/busctl/busctl.c +++ b/src/busctl/busctl.c @@ -1,6 +1,5 @@ /* SPDX-License-Identifier: LGPL-2.1-or-later */ -#include #include #include "sd-bus.h" @@ -9,6 +8,7 @@ #include "sd-json.h" #include "alloc-util.h" +#include "ansi-color.h" #include "bitfield.h" #include "build.h" #include "bus-dump.h" @@ -26,16 +26,17 @@ #include "fileio.h" #include "format-table.h" #include "glyph-util.h" +#include "help-util.h" #include "log.h" #include "logarithm.h" #include "main-func.h" #include "memstream-util.h" +#include "options.h" #include "os-util.h" #include "pager.h" #include "parse-argument.h" #include "parse-util.h" #include "path-util.h" -#include "pretty-print.h" #include "runtime-scope.h" #include "set.h" #include "string-util.h" @@ -175,7 +176,9 @@ static void notify_bus_error(const sd_bus_error *error) { (void) sd_notifyf(/* unset_environment= */ false, "BUSERROR=%s", error->name); } -static int verb_list_bus_names(int argc, char *argv[], uintptr_t _data, void *userdata) { +VERB(verb_list, "list", NULL, VERB_ANY, 1, VERB_DEFAULT, + "List bus names"); +static int verb_list(int argc, char *argv[], uintptr_t _data, void *userdata) { _cleanup_strv_free_ char **acquired = NULL, **activatable = NULL; _cleanup_(sd_bus_flush_close_unrefp) sd_bus *bus = NULL; _cleanup_hashmap_free_ Hashmap *names = NULL; @@ -537,6 +540,8 @@ static int tree_one(sd_bus *bus, const char *service) { return r; } +VERB(verb_tree, "tree", "[SERVICE…]", VERB_ANY, VERB_ANY, 0, + "Show object tree of service"); static int verb_tree(int argc, char *argv[], uintptr_t _data, void *userdata) { _cleanup_(sd_bus_flush_close_unrefp) sd_bus *bus = NULL; int r; @@ -996,6 +1001,8 @@ static int members_flags_to_string(const Member *m, char **ret) { return 0; } +VERB(verb_introspect, "introspect", "SERVICE OBJECT [INTERFACE]", 3, 4, 0, + "Introspect an object"); static int verb_introspect(int argc, char *argv[], uintptr_t _data, void *userdata) { static const XMLIntrospectOps ops = { .on_interface = on_interface, @@ -1377,10 +1384,14 @@ static int monitor(int argc, char **argv, int (*dump)(sd_bus_message *m, FILE *f } } +VERB(verb_monitor, "monitor", "[SERVICE…]", VERB_ANY, VERB_ANY, 0, + "Show bus traffic"); static int verb_monitor(int argc, char *argv[], uintptr_t _data, void *userdata) { return monitor(argc, argv, sd_json_format_enabled(arg_json_format_flags) ? message_json : message_dump); } +VERB(verb_capture, "capture", "[SERVICE…]", VERB_ANY, VERB_ANY, 0, + "Capture bus traffic as pcap"); static int verb_capture(int argc, char *argv[], uintptr_t _data, void *userdata) { _cleanup_free_ char *osname = NULL; static const char info[] = @@ -1408,6 +1419,8 @@ static int verb_capture(int argc, char *argv[], uintptr_t _data, void *userdata) return r; } +VERB(verb_status, "status", "[SERVICE]", VERB_ANY, 2, 0, + "Show bus service, process, or bus owner credentials"); static int verb_status(int argc, char *argv[], uintptr_t _data, void *userdata) { _cleanup_(sd_bus_flush_close_unrefp) sd_bus *bus = NULL; _cleanup_(sd_bus_creds_unrefp) sd_bus_creds *creds = NULL; @@ -1778,6 +1791,8 @@ static int bus_message_dump(sd_bus_message *m, uint64_t flags) { return 0; } +VERB(verb_call, "call", "SERVICE OBJECT INTERFACE METHOD [SIGNATURE [ARGUMENT…]]", 5, VERB_ANY, 0, + "Call a method"); static int verb_call(int argc, char *argv[], uintptr_t _data, void *userdata) { _cleanup_(sd_bus_flush_close_unrefp) sd_bus *bus = NULL; _cleanup_(sd_bus_error_free) sd_bus_error error = SD_BUS_ERROR_NULL; @@ -1845,7 +1860,9 @@ static int verb_call(int argc, char *argv[], uintptr_t _data, void *userdata) { return bus_message_dump(reply, /* flags= */ 0); } -static int verb_emit_signal(int argc, char *argv[], uintptr_t _data, void *userdata) { +VERB(verb_emit, "emit", "OBJECT INTERFACE SIGNAL [SIGNATURE [ARGUMENT…]]", 4, VERB_ANY, 0, + "Emit a signal"); +static int verb_emit(int argc, char *argv[], uintptr_t _data, void *userdata) { _cleanup_(sd_bus_flush_close_unrefp) sd_bus *bus = NULL; _cleanup_(sd_bus_message_unrefp) sd_bus_message *m = NULL; _cleanup_fdset_free_ FDSet *passed_fdset = NULL; @@ -1890,6 +1907,8 @@ static int verb_emit_signal(int argc, char *argv[], uintptr_t _data, void *userd return 0; } +VERB(verb_get_property, "get-property", "SERVICE OBJECT INTERFACE PROPERTY…", 5, VERB_ANY, 0, + "Get property value"); static int verb_get_property(int argc, char *argv[], uintptr_t _data, void *userdata) { _cleanup_(sd_bus_flush_close_unrefp) sd_bus *bus = NULL; _cleanup_(sd_bus_error_free) sd_bus_error error = SD_BUS_ERROR_NULL; @@ -1948,7 +1967,9 @@ static int on_bus_signal(sd_bus_message *msg, void *userdata, sd_bus_error *ret_ return 0; } -static int verb_wait_signal(int argc, char *argv[], uintptr_t _data, void *userdata) { +VERB(verb_wait, "wait", "[SERVICE] OBJECT INTERFACE SIGNAL", 4, 5, 0, + "Wait for a signal"); +static int verb_wait(int argc, char *argv[], uintptr_t _data, void *userdata) { _cleanup_(sd_bus_flush_close_unrefp) sd_bus *bus = NULL; _cleanup_(sd_event_unrefp) sd_event *e = NULL; _cleanup_(sd_event_source_unrefp) sd_event_source *timer = NULL; @@ -1996,6 +2017,8 @@ static int verb_wait_signal(int argc, char *argv[], uintptr_t _data, void *userd return sd_event_loop(e); } +VERB(verb_set_property, "set-property", "SERVICE OBJECT INTERFACE PROPERTY SIGNATURE ARGUMENT…", 6, VERB_ANY, 0, + "Set property value"); static int verb_set_property(int argc, char *argv[], uintptr_t _data, void *userdata) { _cleanup_(sd_bus_flush_close_unrefp) sd_bus *bus = NULL; _cleanup_(sd_bus_message_unrefp) sd_bus_message *m = NULL; @@ -2052,235 +2075,131 @@ static int verb_set_property(int argc, char *argv[], uintptr_t _data, void *user } static int help(void) { - _cleanup_free_ char *link = NULL; + _cleanup_(table_unrefp) Table *options = NULL, *verbs = NULL; int r; - r = terminal_urlify_man("busctl", "1", &link); - if (r < 0) - return log_oom(); - pager_open(arg_pager_flags); - printf("%1$s [OPTIONS...] COMMAND ...\n\n" - "%5$sIntrospect the D-Bus IPC bus.%6$s\n" - "\n%3$sCommands%4$s:\n" - " list List bus names\n" - " status [SERVICE] Show bus service, process or bus owner credentials\n" - " monitor [SERVICE...] Show bus traffic\n" - " capture [SERVICE...] Capture bus traffic as pcap\n" - " tree [SERVICE...] Show object tree of service\n" - " introspect SERVICE OBJECT [INTERFACE]\n" - " call SERVICE OBJECT INTERFACE METHOD [SIGNATURE [ARGUMENT...]]\n" - " Call a method\n" - " emit OBJECT INTERFACE SIGNAL [SIGNATURE [ARGUMENT...]]\n" - " Emit a signal\n" - " wait OBJECT INTERFACE SIGNAL\n" - " Wait for a signal\n" - " get-property SERVICE OBJECT INTERFACE PROPERTY...\n" - " Get property value\n" - " set-property SERVICE OBJECT INTERFACE PROPERTY SIGNATURE ARGUMENT...\n" - " Set property value\n" - " help Show this help\n" - "\n%3$sOptions:%4$s\n" - " -h --help Show this help\n" - " --version Show package version\n" - " --no-pager Do not pipe output into a pager\n" - " --no-legend Do not show the headers and footers\n" - " -l --full Do not ellipsize output\n" - " --system Connect to system bus\n" - " --user Connect to user bus\n" - " -H --host=[USER@]HOST Operate on remote host\n" - " -M --machine=CONTAINER Operate on local container\n" - " --address=ADDRESS Connect to bus specified by address\n" - " --show-machine Show machine ID column in list\n" - " --unique Only show unique names\n" - " --acquired Only show acquired names\n" - " --activatable Only show activatable names\n" - " --match=MATCH Only show matching messages\n" - " --size=SIZE Maximum length of captured packet\n" - " --list Don't show tree, but simple object path list\n" - " -q --quiet Don't show method call reply\n" - " --verbose Show result values in long format\n" - " --json=MODE Output as JSON\n" - " -j Same as --json=pretty on tty, --json=short otherwise\n" - " --xml-interface Dump the XML description in introspect command\n" - " --expect-reply=BOOL Expect a method call reply\n" - " --auto-start=BOOL Auto-start destination service\n" - " --allow-interactive-authorization=BOOL\n" - " Allow interactive authorization for operation\n" - " --timeout=SECS Maximum time to wait for method call completion\n" - " --augment-creds=BOOL Extend credential data with data read from /proc/$PID\n" - " --watch-bind=BOOL Wait for bus AF_UNIX socket to be bound in the file\n" - " system\n" - " --destination=SERVICE Destination service of a signal\n" - " -N --limit-messages=NUMBER\n" - " Stop monitoring after receiving the specified number\n" - " of messages\n" - "\nSee the %2$s for details.\n", - program_invocation_short_name, - link, - ansi_underline(), - ansi_normal(), - ansi_highlight(), - ansi_normal()); + r = verbs_get_help_table(&verbs); + if (r < 0) + return r; + + r = option_parser_get_help_table(&options); + if (r < 0) + return r; + + /* Note: column widths are not synced, because the verbs table is very wide. */ + + help_cmdline("[OPTIONS…] COMMAND …"); + help_abstract("Introspect the D-Bus IPC bus."); + + help_section("Commands"); + r = table_print_or_warn(verbs); + if (r < 0) + return r; + + help_section("Options"); + r = table_print_or_warn(options); + if (r < 0) + return r; + + help_man_page_reference("busctl", "1"); return 0; } -static int verb_help(int argc, char *argv[], uintptr_t _data, void *userdata) { - return help(); -} - -static int parse_argv(int argc, char *argv[]) { - - enum { - ARG_VERSION = 0x100, - ARG_NO_PAGER, - ARG_NO_LEGEND, - ARG_SYSTEM, - ARG_USER, - ARG_ADDRESS, - ARG_MATCH, - ARG_SHOW_MACHINE, - ARG_UNIQUE, - ARG_ACQUIRED, - ARG_ACTIVATABLE, - ARG_SIZE, - ARG_LIST, - ARG_VERBOSE, - ARG_XML_INTERFACE, - ARG_EXPECT_REPLY, - ARG_AUTO_START, - ARG_ALLOW_INTERACTIVE_AUTHORIZATION, - ARG_TIMEOUT, - ARG_AUGMENT_CREDS, - ARG_WATCH_BIND, - ARG_JSON, - ARG_DESTINATION, - }; - - static const struct option options[] = { - { "help", no_argument, NULL, 'h' }, - { "version", no_argument, NULL, ARG_VERSION }, - { "no-pager", no_argument, NULL, ARG_NO_PAGER }, - { "no-legend", no_argument, NULL, ARG_NO_LEGEND }, - { "full", no_argument, NULL, 'l' }, - { "system", no_argument, NULL, ARG_SYSTEM }, - { "user", no_argument, NULL, ARG_USER }, - { "address", required_argument, NULL, ARG_ADDRESS }, - { "show-machine", no_argument, NULL, ARG_SHOW_MACHINE }, - { "unique", no_argument, NULL, ARG_UNIQUE }, - { "acquired", no_argument, NULL, ARG_ACQUIRED }, - { "activatable", no_argument, NULL, ARG_ACTIVATABLE }, - { "match", required_argument, NULL, ARG_MATCH }, - { "host", required_argument, NULL, 'H' }, - { "machine", required_argument, NULL, 'M' }, - { "capsule", required_argument, NULL, 'C' }, - { "size", required_argument, NULL, ARG_SIZE }, - { "list", no_argument, NULL, ARG_LIST }, - { "quiet", no_argument, NULL, 'q' }, - { "verbose", no_argument, NULL, ARG_VERBOSE }, - { "xml-interface", no_argument, NULL, ARG_XML_INTERFACE }, - { "expect-reply", required_argument, NULL, ARG_EXPECT_REPLY }, - { "auto-start", required_argument, NULL, ARG_AUTO_START }, - { "allow-interactive-authorization", required_argument, NULL, ARG_ALLOW_INTERACTIVE_AUTHORIZATION }, - { "timeout", required_argument, NULL, ARG_TIMEOUT }, - { "augment-creds", required_argument, NULL, ARG_AUGMENT_CREDS }, - { "watch-bind", required_argument, NULL, ARG_WATCH_BIND }, - { "json", required_argument, NULL, ARG_JSON }, - { "destination", required_argument, NULL, ARG_DESTINATION }, - { "limit-messages", required_argument, NULL, 'N' }, - {}, - }; - - int c, r; +VERB_COMMON_HELP(help); +static int parse_argv(int argc, char *argv[], char ***remaining_args) { assert(argc >= 0); assert(argv); + assert(remaining_args); - while ((c = getopt_long(argc, argv, "hH:M:C:J:qjlN:", options, NULL)) >= 0) + OptionParser opts = { argc, argv }; + int r; + FOREACH_OPTION_OR_RETURN(c, &opts) switch (c) { - case 'h': + OPTION_COMMON_HELP: return help(); - case ARG_VERSION: + OPTION_COMMON_VERSION: return version(); - case ARG_NO_PAGER: + OPTION_COMMON_NO_PAGER: arg_pager_flags |= PAGER_DISABLE; break; - case ARG_NO_LEGEND: + OPTION_COMMON_NO_LEGEND: arg_legend = false; break; - case 'l': + OPTION('l', "full", NULL, "Do not ellipsize output"): arg_full = true; break; - case ARG_SYSTEM: + OPTION_LONG("system", NULL, "Connect to system bus"): arg_runtime_scope = RUNTIME_SCOPE_SYSTEM; break; - case ARG_USER: + OPTION_LONG("user", NULL, "Connect to user bus"): arg_runtime_scope = RUNTIME_SCOPE_USER; break; - case 'H': + OPTION_COMMON_HOST: arg_transport = BUS_TRANSPORT_REMOTE; - arg_host = optarg; + arg_host = opts.arg; break; - case 'M': - r = parse_machine_argument(optarg, &arg_host, &arg_transport); + OPTION_COMMON_MACHINE: + r = parse_machine_argument(opts.arg, &arg_host, &arg_transport); if (r < 0) return r; break; - case 'C': - r = capsule_name_is_valid(optarg); + OPTION('C', "capsule", "NAME", "Operate on capsule"): + r = capsule_name_is_valid(opts.arg); if (r < 0) - return log_error_errno(r, "Unable to validate capsule name '%s': %m", optarg); + return log_error_errno(r, "Unable to validate capsule name '%s': %m", opts.arg); if (r == 0) - return log_error_errno(SYNTHETIC_ERRNO(EINVAL), "Invalid capsule name: %s", optarg); + return log_error_errno(SYNTHETIC_ERRNO(EINVAL), "Invalid capsule name: %s", opts.arg); - arg_host = optarg; + arg_host = opts.arg; arg_transport = BUS_TRANSPORT_CAPSULE; break; - case ARG_ADDRESS: - arg_address = optarg; + OPTION_LONG("address", "ADDRESS", "Connect to bus specified by address"): + arg_address = opts.arg; break; - case ARG_SHOW_MACHINE: + OPTION_LONG("show-machine", NULL, "Show machine ID column in list"): arg_show_machine = true; break; - case ARG_UNIQUE: + OPTION_LONG("unique", NULL, "Only show unique names"): arg_unique = true; break; - case ARG_ACQUIRED: + OPTION_LONG("acquired", NULL, "Only show acquired names"): arg_acquired = true; break; - case ARG_ACTIVATABLE: + OPTION_LONG("activatable", NULL, "Only show activatable names"): arg_activatable = true; break; - case ARG_MATCH: - if (strv_extend(&arg_matches, optarg) < 0) + OPTION_LONG("match", "MATCH", "Only show matching messages"): + if (strv_extend(&arg_matches, opts.arg) < 0) return log_oom(); break; - case ARG_SIZE: { + OPTION_LONG("size", "SIZE", "Maximum length of captured packet"): { uint64_t sz; - r = parse_size(optarg, 1024, &sz); + r = parse_size(opts.arg, 1024, &sz); if (r < 0) - return log_error_errno(r, "Failed to parse size '%s': %m", optarg); + return log_error_errno(r, "Failed to parse size '%s': %m", opts.arg); if ((uint64_t) (size_t) sz != sz) return log_error_errno(SYNTHETIC_ERRNO(E2BIG), @@ -2290,145 +2209,123 @@ static int parse_argv(int argc, char *argv[]) { break; } - case ARG_LIST: + OPTION_LONG("list", NULL, "Don't show tree, but simple object path list"): arg_list = true; break; - case 'q': + OPTION('q', "quiet", NULL, "Don't show method call reply"): arg_quiet = true; break; - case ARG_VERBOSE: + OPTION_LONG("verbose", NULL, "Show result values in long format"): arg_verbose = true; break; - case ARG_JSON: - r = parse_json_argument(optarg, &arg_json_format_flags); + OPTION_COMMON_JSON: + r = parse_json_argument(opts.arg, &arg_json_format_flags); if (r <= 0) return r; - break; - case 'j': + OPTION_COMMON_LOWERCASE_J: arg_json_format_flags = SD_JSON_FORMAT_PRETTY_AUTO|SD_JSON_FORMAT_COLOR_AUTO; break; - case ARG_XML_INTERFACE: + OPTION_LONG("xml-interface", NULL, "Dump the XML description in introspect command"): arg_xml_interface = true; break; - case ARG_EXPECT_REPLY: - r = parse_boolean_argument("--expect-reply=", optarg, &arg_expect_reply); + OPTION_LONG("expect-reply", "BOOL", "Expect a method call reply"): + r = parse_boolean_argument("--expect-reply=", opts.arg, &arg_expect_reply); if (r < 0) return r; break; - case ARG_AUTO_START: - r = parse_boolean_argument("--auto-start=", optarg, &arg_auto_start); + OPTION_LONG("auto-start", "BOOL", "Auto-start destination service"): + r = parse_boolean_argument("--auto-start=", opts.arg, &arg_auto_start); if (r < 0) return r; break; - case ARG_ALLOW_INTERACTIVE_AUTHORIZATION: - r = parse_boolean_argument("--allow-interactive-authorization=", optarg, + OPTION_LONG("allow-interactive-authorization", "", + "Allow interactive authorization for operation"): + r = parse_boolean_argument("--allow-interactive-authorization=", opts.arg, &arg_allow_interactive_authorization); if (r < 0) return r; break; - case ARG_TIMEOUT: - if (isempty(optarg)) { + OPTION_LONG("timeout", "SECS", "Maximum time to wait for method call completion"): + if (isempty(opts.arg)) { arg_timeout = 0; /* Reset to default */ break; } - r = parse_sec(optarg, &arg_timeout); + r = parse_sec(opts.arg, &arg_timeout); if (r < 0) - return log_error_errno(r, "Failed to parse --timeout= parameter '%s': %m", optarg); - + return log_error_errno(r, "Failed to parse --timeout= parameter '%s': %m", opts.arg); break; - case ARG_AUGMENT_CREDS: - r = parse_boolean_argument("--augment-creds=", optarg, &arg_augment_creds); + OPTION_LONG("augment-creds", "BOOL", + "Extend credential data with data read from /proc/$PID"): + r = parse_boolean_argument("--augment-creds=", opts.arg, &arg_augment_creds); if (r < 0) return r; break; - case ARG_WATCH_BIND: - r = parse_boolean_argument("--watch-bind=", optarg, &arg_watch_bind); + OPTION_LONG("watch-bind", "BOOL", + "Wait for bus AF_UNIX socket to be bound in the file system"): + r = parse_boolean_argument("--watch-bind=", opts.arg, &arg_watch_bind); if (r < 0) return r; break; - case ARG_DESTINATION: - arg_destination = optarg; + OPTION_LONG("destination", "SERVICE", "Destination service of a signal"): + arg_destination = opts.arg; break; - case 'N': - if (isempty(optarg)) { + OPTION('N', "limit-messages", "NUMBER", + "Stop monitoring after receiving the specified number of messages"): + if (isempty(opts.arg)) { /* Reset to default */ arg_limit_messages = UINT64_MAX; arg_limit_signals = 1; break; } - if (streq(optarg, "infinity")) { + if (streq(opts.arg, "infinity")) { arg_limit_signals = arg_limit_messages = UINT64_MAX; break; } - r = safe_atou64(optarg, &arg_limit_messages); + r = safe_atou64(opts.arg, &arg_limit_messages); if (r < 0) - return log_error_errno(r, "Failed to parse --limit-messages= parameter: %s", optarg); + return log_error_errno(r, "Failed to parse --limit-messages= parameter: %s", opts.arg); if (arg_limit_messages == 0) return log_error_errno(SYNTHETIC_ERRNO(EINVAL), "--limit-messages= parameter cannot be 0"); arg_limit_signals = arg_limit_messages; break; - - case '?': - return -EINVAL; - - default: - assert_not_reached(); } if (arg_full < 0) arg_full = terminal_is_dumb(); + *remaining_args = option_parser_get_args(&opts); return 1; } -static int busctl_main(int argc, char *argv[]) { - static const Verb verbs[] = { - { "list", VERB_ANY, 1, VERB_DEFAULT, verb_list_bus_names }, - { "status", VERB_ANY, 2, 0, verb_status }, - { "monitor", VERB_ANY, VERB_ANY, 0, verb_monitor }, - { "capture", VERB_ANY, VERB_ANY, 0, verb_capture }, - { "tree", VERB_ANY, VERB_ANY, 0, verb_tree }, - { "introspect", 3, 4, 0, verb_introspect }, - { "call", 5, VERB_ANY, 0, verb_call }, - { "emit", 4, VERB_ANY, 0, verb_emit_signal }, - { "wait", 4, 5, 0, verb_wait_signal }, - { "get-property", 5, VERB_ANY, 0, verb_get_property }, - { "set-property", 6, VERB_ANY, 0, verb_set_property }, - { "help", VERB_ANY, VERB_ANY, 0, verb_help }, - {} - }; - - return dispatch_verb(argc, argv, verbs, NULL); -} - static int run(int argc, char *argv[]) { int r; log_setup(); - r = parse_argv(argc, argv); + char **args = NULL; + r = parse_argv(argc, argv, &args); if (r <= 0) return r; - return busctl_main(argc, argv); + return dispatch_verb_with_args(args, NULL); } DEFINE_MAIN_FUNCTION(run); From 587c522487664a1b3b42765c61b38aaf43e812f4 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Zbigniew=20J=C4=99drzejewski-Szmek?= Date: Thu, 23 Apr 2026 21:43:19 +0200 Subject: [PATCH 8/8] various: fix duplicated logging from parse_path_argument As pointed out in review, parse_path_argument can fail for non-oom reasons. But the function already logs, so the correct thing to do is to just propagate the error. --- src/bootctl/bootctl-install.c | 2 +- src/home/homectl.c | 6 +++--- src/kernel-install/kernel-install.c | 4 ++-- src/keyutil/keyutil.c | 4 ++-- src/measure/measure-tool.c | 2 +- src/sbsign/sbsign.c | 2 +- 6 files changed, 10 insertions(+), 10 deletions(-) diff --git a/src/bootctl/bootctl-install.c b/src/bootctl/bootctl-install.c index 20958d0b0b..2ad76264fa 100644 --- a/src/bootctl/bootctl-install.c +++ b/src/bootctl/bootctl-install.c @@ -1509,7 +1509,7 @@ static int load_secure_boot_auto_enroll( if (arg_private_key_source_type == OPENSSL_KEY_SOURCE_FILE) { r = parse_path_argument(arg_private_key, /* suppress_root= */ false, &arg_private_key); if (r < 0) - return log_error_errno(r, "Failed to parse private key path %s: %m", arg_private_key); + return r; } r = openssl_load_private_key( diff --git a/src/home/homectl.c b/src/home/homectl.c index 80ee6e2155..e9161dfb75 100644 --- a/src/home/homectl.c +++ b/src/home/homectl.c @@ -4648,7 +4648,7 @@ static int parse_argv(int argc, char *argv[]) { if (!eq) { /* --blob=/some/path replaces the blob dir */ r = parse_path_argument(optarg, /* suppress_root= */ false, &arg_blob_dir); if (r < 0) - return log_error_errno(r, "Failed to parse path %s: %m", optarg); + return r; break; } @@ -4664,7 +4664,7 @@ static int parse_argv(int argc, char *argv[]) { r = parse_path_argument(eq + 1, /* suppress_root= */ false, &path); if (r < 0) - return log_error_errno(r, "Failed to parse path %s: %m", eq + 1); + return r; } else { const char *well_known_filename = c == ARG_AVATAR ? "avatar" : @@ -4678,7 +4678,7 @@ static int parse_argv(int argc, char *argv[]) { r = parse_path_argument(optarg, /* suppress_root= */ false, &path); if (r < 0) - return log_error_errno(r, "Failed to parse path %s: %m", optarg); + return r; } if (path) { diff --git a/src/kernel-install/kernel-install.c b/src/kernel-install/kernel-install.c index 0bd4e84da0..27e1a7426e 100644 --- a/src/kernel-install/kernel-install.c +++ b/src/kernel-install/kernel-install.c @@ -1604,13 +1604,13 @@ static int parse_argv(int argc, char *argv[], char ***remaining_args) { OPTION_LONG("esp-path", "PATH", "Path to the EFI System Partition (ESP)"): r = parse_path_argument(opts.arg, /* suppress_root= */ false, &arg_esp_path); if (r < 0) - return log_oom(); + return r; break; OPTION_LONG("boot-path", "PATH", "Path to the $BOOT partition"): r = parse_path_argument(opts.arg, /* suppress_root= */ false, &arg_xbootldr_path); if (r < 0) - return log_oom(); + return r; break; OPTION_COMMON_MAKE_ENTRY_DIRECTORY: diff --git a/src/keyutil/keyutil.c b/src/keyutil/keyutil.c index 2a66fabb19..abdec1591d 100644 --- a/src/keyutil/keyutil.c +++ b/src/keyutil/keyutil.c @@ -193,7 +193,7 @@ static int verb_validate(int argc, char *argv[], uintptr_t _data, void *userdata if (arg_private_key_source_type == OPENSSL_KEY_SOURCE_FILE) { r = parse_path_argument(arg_private_key, /* suppress_root= */ false, &arg_private_key); if (r < 0) - return log_error_errno(r, "Failed to parse private key path %s: %m", arg_private_key); + return r; } r = openssl_load_private_key( @@ -259,7 +259,7 @@ static int verb_extract_public(int argc, char *argv[], uintptr_t _data, void *us if (arg_private_key_source_type == OPENSSL_KEY_SOURCE_FILE) { r = parse_path_argument(arg_private_key, /* suppress_root= */ false, &arg_private_key); if (r < 0) - return log_error_errno(r, "Failed to parse private key path %s: %m", arg_private_key); + return r; } r = openssl_load_private_key( diff --git a/src/measure/measure-tool.c b/src/measure/measure-tool.c index eeb001f3fe..d5bb5bea23 100644 --- a/src/measure/measure-tool.c +++ b/src/measure/measure-tool.c @@ -960,7 +960,7 @@ static int build_policy_digest(bool sign) { if (arg_private_key_source_type == OPENSSL_KEY_SOURCE_FILE) { r = parse_path_argument(arg_private_key, /* suppress_root= */ false, &arg_private_key); if (r < 0) - return log_error_errno(r, "Failed to parse private key path %s: %m", arg_private_key); + return r; } r = openssl_load_private_key( diff --git a/src/sbsign/sbsign.c b/src/sbsign/sbsign.c index f5a88b2849..9d163624a4 100644 --- a/src/sbsign/sbsign.c +++ b/src/sbsign/sbsign.c @@ -461,7 +461,7 @@ static int verb_sign(int argc, char *argv[], uintptr_t _data, void *userdata) { if (arg_private_key_source_type == OPENSSL_KEY_SOURCE_FILE) { r = parse_path_argument(arg_private_key, /* suppress_root= */ false, &arg_private_key); if (r < 0) - return log_error_errno(r, "Failed to parse private key path %s: %m", arg_private_key); + return r; } r = openssl_load_private_key(