From 805d3f5c53298dd14770f40d4315f6c0d5ebbadb Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Zbigniew=20J=C4=99drzejewski-Szmek?= Date: Fri, 27 Mar 2026 09:17:51 +0100 Subject: [PATCH 01/11] shared/verbs: extend comment This was suggested in one of the reviews but I forgot to push the change. --- src/shared/verbs.h | 3 ++- 1 file changed, 2 insertions(+), 1 deletion(-) diff --git a/src/shared/verbs.h b/src/shared/verbs.h index 6b380041b8..cd18359cd7 100644 --- a/src/shared/verbs.h +++ b/src/shared/verbs.h @@ -52,7 +52,8 @@ typedef struct { #define VERB_NOARG(d, v, h) \ VERB(d, v, /* a= */ NULL, /* amin= */ VERB_ANY, /* amax= */ 1, /* f= */ 0, h) -/* Magic entry in the table (which will not be returned) that designates the start of the group . */ +/* Magic entry in the table (which will not be returned) that designates the start of the group . + * The macro works as a separator between groups and must be between other VERB* stanzas. */ #define VERB_GROUP(gr) \ _VERB_DATA(/* d= */ NULL, /* v= */ gr, /* a= */ NULL, /* amin= */ 0, /* amax= */ 0, \ /* f= */ VERB_GROUP_MARKER, /* dat= */ 0, /* h= */ NULL) From b4e0ff0874fb6128fc4bd0610e94029ce93b9454 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Zbigniew=20J=C4=99drzejewski-Szmek?= Date: Wed, 1 Apr 2026 17:31:18 +0200 Subject: [PATCH 02/11] shared/options: extend comment --- src/shared/options.c | 3 ++- 1 file changed, 2 insertions(+), 1 deletion(-) diff --git a/src/shared/options.c b/src/shared/options.c index 853df0d38d..e6b82fe34d 100644 --- a/src/shared/options.c +++ b/src/shared/options.c @@ -238,7 +238,8 @@ char** option_parser_get_args(const OptionParser *state) { /* Returns positional args as a strv. * If "--" was found, it has been moved before state->positional_offset. * The array is only valid, i.e. clean without any options, after parsing - * has naturally finished. */ + * has naturally finished. The array that is returned is a slice of the + * original argv array, so it must not be freed or modified. */ assert(state->optind > 0); assert(state->optind == state->argc || state->parsing_stopped); From f833b092d8075e3c0d03a05f818f454e6fbd6936 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Zbigniew=20J=C4=99drzejewski-Szmek?= Date: Fri, 3 Apr 2026 10:27:43 +0200 Subject: [PATCH 03/11] analyze: use table_print_with_pager in one more place I guess this wasn't converted previously because verb_blame doesn't support json output, the flags that are passed atm cannot contain real json flags. That's OK, we can still use table_print_with_pager. --- src/analyze/analyze-blame.c | 10 +--------- 1 file changed, 1 insertion(+), 9 deletions(-) diff --git a/src/analyze/analyze-blame.c b/src/analyze/analyze-blame.c index 8651f2586a..24b29a26a1 100644 --- a/src/analyze/analyze-blame.c +++ b/src/analyze/analyze-blame.c @@ -28,8 +28,6 @@ int verb_blame(int argc, char *argv[], uintptr_t _data, void *userdata) { if (!table) return log_oom(); - table_set_header(table, false); - assert_se(cell = table_get_cell(table, 0, 0)); r = table_set_ellipsize_percent(table, cell, 100); if (r < 0) @@ -63,11 +61,5 @@ int verb_blame(int argc, char *argv[], uintptr_t _data, void *userdata) { return table_log_add_error(r); } - pager_open(arg_pager_flags); - - r = table_print(table, NULL); - if (r < 0) - return r; - - return 0; + return table_print_with_pager(table, arg_json_format_flags, arg_pager_flags, /* show_header= */ false); } From 16a43412b9fbe3a4af8cafff40a0c2fedf34efc2 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Zbigniew=20J=C4=99drzejewski-Szmek?= Date: Fri, 3 Apr 2026 10:53:53 +0200 Subject: [PATCH 04/11] busctl: use table_print_with_pager in one more place --- src/busctl/busctl.c | 8 +------- 1 file changed, 1 insertion(+), 7 deletions(-) diff --git a/src/busctl/busctl.c b/src/busctl/busctl.c index aa04c9bbf0..a895c3fe91 100644 --- a/src/busctl/busctl.c +++ b/src/busctl/busctl.c @@ -1188,13 +1188,7 @@ static int verb_introspect(int argc, char *argv[], uintptr_t _data, void *userda return table_log_add_error(r); } - pager_open(arg_pager_flags); - - r = table_print(table, NULL); - if (r < 0) - return table_log_print_error(r); - - return 0; + return table_print_with_pager(table, arg_json_format_flags, arg_pager_flags, arg_legend); } static int message_dump(sd_bus_message *m, FILE *f) { From 0ce5a8fb4ee182da1967c8d99ce100f1c484ad15 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Zbigniew=20J=C4=99drzejewski-Szmek?= Date: Fri, 3 Apr 2026 10:59:18 +0200 Subject: [PATCH 05/11] tree-wide: drop flush&check step after table printing Almost all callers of table_print() specify stdout or NULL (equivalent to stdout) as the output stream. Simplify things by not requiring the the stream to be specified. In almost all cases, the printing of the table is surrounded by normal printfs() that don't do explicit flushing and for which we don't check the output stream status. Let's simplify most callers and skip this step. The reason is not so much to avoid the extra step itself, but instead to avoid the _handling_ of the potential failure. We generally only want to print an error message for ENOMEM and other "internal" errors, so strictly speaking we should filter out the errors from the stream. By skipping the flush&check step we implicitly do this. --- src/ac-power/ac-power.c | 2 +- src/analyze/analyze-calendar.c | 2 +- src/analyze/analyze-image-policy.c | 2 +- src/analyze/analyze-inspect-elf.c | 2 +- src/analyze/analyze-plot.c | 2 +- src/analyze/analyze-timespan.c | 2 +- src/analyze/analyze-timestamp.c | 2 +- src/ask-password/ask-password.c | 2 +- src/binfmt/binfmt.c | 2 +- src/bless-boot/bless-boot.c | 4 ++-- src/cryptenroll/cryptenroll-list.c | 2 +- src/detect-virt/detect-virt.c | 2 +- src/dissect/dissect.c | 6 +++--- src/factory-reset/factory-reset-tool.c | 4 ++-- src/hostname/hostnamectl.c | 6 +++--- src/id128/id128.c | 4 ++-- src/imds/imds-tool.c | 2 +- src/locale/localectl.c | 2 +- src/login/loginctl.c | 6 +++--- src/machine/machinectl.c | 2 +- src/network/networkctl-address-label.c | 2 +- src/network/networkctl-list.c | 2 +- src/network/networkctl-lldp.c | 2 +- src/network/networkctl-status-link.c | 2 +- src/network/networkctl-status-system.c | 2 +- src/notify/notify.c | 2 +- src/portable/portablectl.c | 2 +- src/report/report-basic-server.c | 2 +- src/resolve/resolvectl.c | 6 +++--- src/run/run.c | 2 +- src/shared/format-table.c | 9 ++++++--- src/shared/format-table.h | 6 +++++- src/shared/libfido2-util.c | 2 +- src/shared/parse-argument.c | 2 +- src/shared/pkcs11-util.c | 2 +- src/shared/tpm2-util.c | 2 +- src/systemctl/systemctl-list-jobs.c | 2 +- src/systemctl/systemctl-util.c | 2 +- src/test/test-format-table.c | 2 +- src/timedate/timedatectl.c | 8 ++++---- src/update-done/update-done.c | 2 +- src/validatefs/validatefs.c | 2 +- src/varlinkctl/varlinkctl.c | 2 +- src/vpick/vpick-tool.c | 2 +- 44 files changed, 67 insertions(+), 60 deletions(-) diff --git a/src/ac-power/ac-power.c b/src/ac-power/ac-power.c index 1ca1048c5a..b153194cdb 100644 --- a/src/ac-power/ac-power.c +++ b/src/ac-power/ac-power.c @@ -37,7 +37,7 @@ static int help(void) { program_invocation_short_name, ansi_highlight(), ansi_normal()); - table_print(options, stdout); + table_print(options); printf("\nSee the %s for details.\n", link); return 0; diff --git a/src/analyze/analyze-calendar.c b/src/analyze/analyze-calendar.c index ac0b2da7d8..ea99f3871e 100644 --- a/src/analyze/analyze-calendar.c +++ b/src/analyze/analyze-calendar.c @@ -119,7 +119,7 @@ static int test_calendar_one(usec_t n, const char *p) { n = next; } - return table_print(table, NULL); + return table_print(table); } int verb_calendar(int argc, char *argv[], uintptr_t _data, void *userdata) { diff --git a/src/analyze/analyze-image-policy.c b/src/analyze/analyze-image-policy.c index 93777c91a1..16e69414c1 100644 --- a/src/analyze/analyze-image-policy.c +++ b/src/analyze/analyze-image-policy.c @@ -157,7 +157,7 @@ int verb_image_policy(int argc, char *argv[], uintptr_t _data, void *userdata) { putc('\n', stdout); - r = table_print(table, NULL); + r = table_print(table); if (r < 0) return r; } diff --git a/src/analyze/analyze-inspect-elf.c b/src/analyze/analyze-inspect-elf.c index 41dabd051c..f4fcc3bd57 100644 --- a/src/analyze/analyze-inspect-elf.c +++ b/src/analyze/analyze-inspect-elf.c @@ -118,7 +118,7 @@ static int analyze_elf(char **filenames, sd_json_format_flags_t json_flags) { if (sd_json_format_enabled(json_flags)) sd_json_variant_dump(package_metadata, json_flags, stdout, NULL); else { - r = table_print(t, NULL); + r = table_print(t); if (r < 0) return table_log_print_error(r); } diff --git a/src/analyze/analyze-plot.c b/src/analyze/analyze-plot.c index 8460757b8a..7f92c1c6bb 100644 --- a/src/analyze/analyze-plot.c +++ b/src/analyze/analyze-plot.c @@ -429,7 +429,7 @@ static int show_table(Table *table, const char *word) { if (sd_json_format_enabled(arg_json_format_flags)) r = table_print_json(table, NULL, arg_json_format_flags | SD_JSON_FORMAT_COLOR_AUTO); else - r = table_print(table, NULL); + r = table_print(table); if (r < 0) return table_log_print_error(r); } diff --git a/src/analyze/analyze-timespan.c b/src/analyze/analyze-timespan.c index fb077617d4..b6ca7bb4af 100644 --- a/src/analyze/analyze-timespan.c +++ b/src/analyze/analyze-timespan.c @@ -55,7 +55,7 @@ int verb_timespan(int argc, char *argv[], uintptr_t _data, void *userdata) { if (r < 0) return table_log_add_error(r); - r = table_print(table, NULL); + r = table_print(table); if (r < 0) return r; diff --git a/src/analyze/analyze-timestamp.c b/src/analyze/analyze-timestamp.c index e7ba6e1bcc..d998ca830a 100644 --- a/src/analyze/analyze-timestamp.c +++ b/src/analyze/analyze-timestamp.c @@ -73,7 +73,7 @@ static int test_timestamp_one(const char *p) { if (r < 0) return table_log_add_error(r); - return table_print(table, NULL); + return table_print(table); } int verb_timestamp(int argc, char *argv[], uintptr_t _data, void *userdata) { diff --git a/src/ask-password/ask-password.c b/src/ask-password/ask-password.c index 2c032c1afb..2ed2be8afe 100644 --- a/src/ask-password/ask-password.c +++ b/src/ask-password/ask-password.c @@ -57,7 +57,7 @@ static int help(void) { program_invocation_short_name, ansi_highlight(), ansi_normal()); - table_print(options, stdout); + table_print(options); printf("\nSee the %s for details.\n", link); return 0; diff --git a/src/binfmt/binfmt.c b/src/binfmt/binfmt.c index 23c09fe349..06fff811bf 100644 --- a/src/binfmt/binfmt.c +++ b/src/binfmt/binfmt.c @@ -126,7 +126,7 @@ static int help(void) { program_invocation_short_name, ansi_highlight(), ansi_normal()); - table_print(options, stdout); + table_print(options); printf("\nSee the %s for details.\n", link); return 0; diff --git a/src/bless-boot/bless-boot.c b/src/bless-boot/bless-boot.c index b82be92dbd..67da021f29 100644 --- a/src/bless-boot/bless-boot.c +++ b/src/bless-boot/bless-boot.c @@ -58,10 +58,10 @@ static int help(void) { program_invocation_short_name, ansi_highlight(), ansi_normal()); - table_print(verbs, stdout); + table_print(verbs); printf("\nOptions:\n"); - table_print(options, stdout); + table_print(options); printf("\nSee the %s for details.\n", link); return 0; diff --git a/src/cryptenroll/cryptenroll-list.c b/src/cryptenroll/cryptenroll-list.c index 32e8c9cf2a..bf9f2a130e 100644 --- a/src/cryptenroll/cryptenroll-list.c +++ b/src/cryptenroll/cryptenroll-list.c @@ -125,7 +125,7 @@ int list_enrolled(struct crypt_device *cd) { return 0; } - r = table_print(t, stdout); + r = table_print(t); if (r < 0) return log_error_errno(r, "Failed to show slot table: %m"); diff --git a/src/detect-virt/detect-virt.c b/src/detect-virt/detect-virt.c index 912f6fbdd6..a8b9739a92 100644 --- a/src/detect-virt/detect-virt.c +++ b/src/detect-virt/detect-virt.c @@ -41,7 +41,7 @@ static int help(void) { program_invocation_short_name, ansi_highlight(), ansi_normal()); - table_print(options, stdout); + table_print(options); printf("\nSee the %s for details.\n", link); return 0; diff --git a/src/dissect/dissect.c b/src/dissect/dissect.c index 26fa1aa1a3..91ad3a4502 100644 --- a/src/dissect/dissect.c +++ b/src/dissect/dissect.c @@ -165,11 +165,11 @@ static int help(void) { ansi_underline(), ansi_normal()); - table_print(options, stdout); + table_print(options); printf("\n%sCommands:%s\n", ansi_underline(), ansi_normal()); - table_print(commands, stdout); + table_print(commands); printf("\nSee the %s for details.\n", link); return 0; @@ -1082,7 +1082,7 @@ static int action_dissect( if (!sd_json_format_enabled(arg_json_format_flags)) { table_set_header(t, arg_legend); - r = table_print(t, NULL); + r = table_print(t); if (r < 0) return table_log_print_error(r); } else { diff --git a/src/factory-reset/factory-reset-tool.c b/src/factory-reset/factory-reset-tool.c index e7eabd8757..c09369e229 100644 --- a/src/factory-reset/factory-reset-tool.c +++ b/src/factory-reset/factory-reset-tool.c @@ -51,10 +51,10 @@ static int help(void) { program_invocation_short_name, ansi_highlight(), ansi_normal()); - table_print(verbs, stdout); + table_print(verbs); printf("\nOptions:\n"); - table_print(options, stdout); + table_print(options); printf("\nSee the %s for details.\n", link); return 0; diff --git a/src/hostname/hostnamectl.c b/src/hostname/hostnamectl.c index 52fa3319d7..52b31a5867 100644 --- a/src/hostname/hostnamectl.c +++ b/src/hostname/hostnamectl.c @@ -375,7 +375,7 @@ static int print_status_info(StatusInfo *i) { } } - r = table_print(table, NULL); + r = table_print(table); if (r < 0) return table_log_print_error(r); @@ -748,10 +748,10 @@ static int help(void) { program_invocation_short_name, ansi_highlight(), ansi_normal()); - table_print(verbs, stdout); + table_print(verbs); printf("\nOptions:\n"); - table_print(options, stdout); + table_print(options); printf("\nSee the %s for details.\n", link); return 0; diff --git a/src/id128/id128.c b/src/id128/id128.c index f23403b3f8..eda117aec0 100644 --- a/src/id128/id128.c +++ b/src/id128/id128.c @@ -216,10 +216,10 @@ static int help(void) { program_invocation_short_name, ansi_highlight(), ansi_normal()); - table_print(verbs, stdout); + table_print(verbs); printf("\nOptions:\n"); - table_print(options, stdout); + table_print(options); printf("\nSee the %s for details.\n", link); return 0; diff --git a/src/imds/imds-tool.c b/src/imds/imds-tool.c index 4ae8dbb33c..61fc82014e 100644 --- a/src/imds/imds-tool.c +++ b/src/imds/imds-tool.c @@ -381,7 +381,7 @@ static int action_summary(sd_varlink *link) { if (table_isempty(table)) return log_error_errno(SYNTHETIC_ERRNO(EOPNOTSUPP), "No well-known IMDS data available."); - r = table_print(table, NULL); + r = table_print(table); if (r < 0) return table_log_print_error(r); diff --git a/src/locale/localectl.c b/src/locale/localectl.c index 65756d26f0..b67a67e73b 100644 --- a/src/locale/localectl.c +++ b/src/locale/localectl.c @@ -144,7 +144,7 @@ static int print_status_info(StatusInfo *i) { return table_log_add_error(r); } - r = table_print(table, NULL); + r = table_print(table); if (r < 0) return table_log_print_error(r); diff --git a/src/login/loginctl.c b/src/login/loginctl.c index a921a4a677..fa1c024481 100644 --- a/src/login/loginctl.c +++ b/src/login/loginctl.c @@ -716,7 +716,7 @@ static int print_session_status_info(sd_bus *bus, const char *path) { /* We don't use the table to show the header, in order to make the width of the column stable. */ printf("%s%s - %s (" UID_FMT ")%s\n", ansi_highlight(), i.id, i.name, i.uid, ansi_normal()); - r = table_print(table, NULL); + r = table_print(table); if (r < 0) return table_log_print_error(r); @@ -821,7 +821,7 @@ static int print_user_status_info(sd_bus *bus, const char *path) { printf("%s%s (" UID_FMT ")%s\n", ansi_highlight(), i.name, i.uid, ansi_normal()); - r = table_print(table, NULL); + r = table_print(table); if (r < 0) return table_log_print_error(r); @@ -896,7 +896,7 @@ static int print_seat_status_info(sd_bus *bus, const char *path) { printf("%s%s%s\n", ansi_highlight(), i.id, ansi_normal()); - r = table_print(table, NULL); + r = table_print(table); if (r < 0) return table_log_print_error(r); diff --git a/src/machine/machinectl.c b/src/machine/machinectl.c index 733b1a19ef..6c684149cb 100644 --- a/src/machine/machinectl.c +++ b/src/machine/machinectl.c @@ -262,7 +262,7 @@ static int show_table(Table *table, const char *word) { if (OUTPUT_MODE_IS_JSON(arg_output)) r = table_print_json(table, NULL, output_mode_to_json_format_flags(arg_output) | SD_JSON_FORMAT_COLOR_AUTO); else - r = table_print(table, NULL); + r = table_print(table); if (r < 0) return table_log_print_error(r); } diff --git a/src/network/networkctl-address-label.c b/src/network/networkctl-address-label.c index f587d0cfbb..04b6d4d236 100644 --- a/src/network/networkctl-address-label.c +++ b/src/network/networkctl-address-label.c @@ -82,7 +82,7 @@ static int dump_address_labels(sd_netlink *rtnl) { return table_log_add_error(r); } - r = table_print(table, NULL); + r = table_print(table); if (r < 0) return table_log_print_error(r); diff --git a/src/network/networkctl-list.c b/src/network/networkctl-list.c index 1ef38a0b85..c30be4a1de 100644 --- a/src/network/networkctl-list.c +++ b/src/network/networkctl-list.c @@ -79,7 +79,7 @@ int verb_list_links(int argc, char *argv[], uintptr_t _data, void *userdata) { return table_log_add_error(r); } - r = table_print(table, NULL); + r = table_print(table); if (r < 0) return table_log_print_error(r); diff --git a/src/network/networkctl-lldp.c b/src/network/networkctl-lldp.c index ddce26e5c4..c91d8fea00 100644 --- a/src/network/networkctl-lldp.c +++ b/src/network/networkctl-lldp.c @@ -303,7 +303,7 @@ int verb_link_lldp_status(int argc, char *argv[], uintptr_t _data, void *userdat } } - r = table_print(table, NULL); + r = table_print(table); if (r < 0) return table_log_print_error(r); diff --git a/src/network/networkctl-status-link.c b/src/network/networkctl-status-link.c index f63ee2d417..9cbf3efb33 100644 --- a/src/network/networkctl-status-link.c +++ b/src/network/networkctl-status-link.c @@ -900,7 +900,7 @@ static int link_status_one( on_color_operational, glyph(GLYPH_BLACK_CIRCLE), off_color_operational, info->ifindex, info->name); - r = table_print(table, NULL); + r = table_print(table); if (r < 0) return table_log_print_error(r); diff --git a/src/network/networkctl-status-system.c b/src/network/networkctl-status-system.c index 20a4c2be91..ce403d6062 100644 --- a/src/network/networkctl-status-system.c +++ b/src/network/networkctl-status-system.c @@ -126,7 +126,7 @@ int system_status(sd_netlink *rtnl, sd_hwdb *hwdb) { on_color_operational, glyph(GLYPH_BLACK_CIRCLE), off_color_operational, strna(netifs_joined)); - r = table_print(table, NULL); + r = table_print(table); if (r < 0) return table_log_print_error(r); diff --git a/src/notify/notify.c b/src/notify/notify.c index a06f5ce773..e425a125ac 100644 --- a/src/notify/notify.c +++ b/src/notify/notify.c @@ -77,7 +77,7 @@ static int help(void) { ansi_highlight(), ansi_normal()); - table_print(options, stdout); + table_print(options); printf("\nSee the %s for details.\n", link); return 0; diff --git a/src/portable/portablectl.c b/src/portable/portablectl.c index 0cd461a997..f5ce3db9c4 100644 --- a/src/portable/portablectl.c +++ b/src/portable/portablectl.c @@ -1075,7 +1075,7 @@ static int verb_list_images(int argc, char *argv[], uintptr_t _data, void *userd table_set_header(table, arg_legend); - r = table_print(table, NULL); + r = table_print(table); if (r < 0) return table_log_print_error(r); } diff --git a/src/report/report-basic-server.c b/src/report/report-basic-server.c index 32ec9b0356..dc482a37ff 100644 --- a/src/report/report-basic-server.c +++ b/src/report/report-basic-server.c @@ -47,7 +47,7 @@ static int help(void) { ansi_normal(), ansi_underline(), ansi_normal()); - table_print(options, stdout); + table_print(options); return 0; } diff --git a/src/resolve/resolvectl.c b/src/resolve/resolvectl.c index e4705a6bcc..fe2b676c7c 100644 --- a/src/resolve/resolvectl.c +++ b/src/resolve/resolvectl.c @@ -1321,7 +1321,7 @@ static int verb_show_statistics(int argc, char *argv[], uintptr_t _data, void *u if (r < 0) return table_log_add_error(r); - r = table_print(table, NULL); + r = table_print(table); if (r < 0) return table_log_print_error(r); @@ -1890,7 +1890,7 @@ static int print_configuration(DNSConfiguration *configuration, StatusMode mode, return table_log_add_error(r); } - r = table_print(table, NULL); + r = table_print(table); if (r < 0) return table_log_print_error(r); @@ -3097,7 +3097,7 @@ static int dump_server_state(sd_json_variant *server) { if (r < 0) return table_log_add_error(r); - r = table_print(table, NULL); + r = table_print(table); if (r < 0) return table_log_print_error(r); diff --git a/src/run/run.c b/src/run/run.c index f1bbbe1f39..88a9f41d69 100644 --- a/src/run/run.c +++ b/src/run/run.c @@ -2437,7 +2437,7 @@ static int run_context_show_result(RunContext *c) { return table_log_add_error(r); } - r = table_print(t, stderr); + r = table_print_full(t, stderr, /* flush= */ true); if (r < 0) return table_log_print_error(r); diff --git a/src/shared/format-table.c b/src/shared/format-table.c index 04552b5b56..718d5c09b8 100644 --- a/src/shared/format-table.c +++ b/src/shared/format-table.c @@ -2217,7 +2217,7 @@ int _table_sync_column_widths(size_t column, Table *a, ...) { return r; } -int table_print(Table *t, FILE *f) { +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; @@ -2637,6 +2637,9 @@ int table_print(Table *t, FILE *f) { } while (more_sublines); } + if (!flush) + return 0; + return fflush_and_check(f); } @@ -2652,7 +2655,7 @@ int table_format(Table *t, char **ret) { if (!f) return -ENOMEM; - r = table_print(t, f); + r = table_print_full(t, f, /* flush= */ true); if (r < 0) return r; @@ -3141,7 +3144,7 @@ int table_print_json(Table *t, FILE *f, sd_json_format_flags_t flags) { assert(t); if (!sd_json_format_enabled(flags)) /* If JSON output is turned off, use regular output */ - return table_print(t, f); + return table_print_full(t, f, /* flush= */ true); if (!f) f = stdout; diff --git a/src/shared/format-table.h b/src/shared/format-table.h index bb5a68b7e9..ee4007394e 100644 --- a/src/shared/format-table.h +++ b/src/shared/format-table.h @@ -147,7 +147,11 @@ int table_set_column_width(Table *t, size_t column, size_t width); int _table_sync_column_widths(size_t column, Table *a, ...); #define table_sync_column_widths(column, a, ...) _table_sync_column_widths(column, a, __VA_ARGS__, NULL) -int table_print(Table *t, FILE *f); +int table_print_full(Table *t, FILE *f, bool flush); +static inline int table_print(Table *t) { + return table_print_full(t, /* f= */ NULL, /* flush= */ false); +} + int table_format(Table *t, char **ret); static inline TableCell* TABLE_HEADER_CELL(size_t i) { diff --git a/src/shared/libfido2-util.c b/src/shared/libfido2-util.c index f4c8ad5c86..7e50d0dd2d 100644 --- a/src/shared/libfido2-util.c +++ b/src/shared/libfido2-util.c @@ -1241,7 +1241,7 @@ int fido2_list_devices(void) { } } - r = table_print(t, stdout); + r = table_print(t); if (r < 0) { log_error_errno(r, "Failed to show device table: %m"); goto finish; diff --git a/src/shared/parse-argument.c b/src/shared/parse-argument.c index 39e5328e70..6ac42c4d88 100644 --- a/src/shared/parse-argument.c +++ b/src/shared/parse-argument.c @@ -128,7 +128,7 @@ int parse_signal_argument(const char *s, int *ret) { return table_log_add_error(r); } - r = table_print(table, NULL); + r = table_print(table); if (r < 0) return table_log_print_error(r); diff --git a/src/shared/pkcs11-util.c b/src/shared/pkcs11-util.c index 1fa0d77d6d..2d8ce29b58 100644 --- a/src/shared/pkcs11-util.c +++ b/src/shared/pkcs11-util.c @@ -1816,7 +1816,7 @@ int pkcs11_list_tokens(void) { return 0; } - r = table_print(t, stdout); + r = table_print(t); if (r < 0) return log_error_errno(r, "Failed to show device table: %m"); diff --git a/src/shared/tpm2-util.c b/src/shared/tpm2-util.c index dc062117b4..a1f05d6397 100644 --- a/src/shared/tpm2-util.c +++ b/src/shared/tpm2-util.c @@ -6584,7 +6584,7 @@ int tpm2_list_devices(bool legend, bool quiet) { return 0; } - r = table_print(t, stdout); + r = table_print(t); if (r < 0) return log_error_errno(r, "Failed to show device table: %m"); diff --git a/src/systemctl/systemctl-list-jobs.c b/src/systemctl/systemctl-list-jobs.c index 5804d32518..bf3f5ed866 100644 --- a/src/systemctl/systemctl-list-jobs.c +++ b/src/systemctl/systemctl-list-jobs.c @@ -115,7 +115,7 @@ static int output_jobs_list(sd_bus *bus, const struct job_info* jobs, unsigned n output_waiting_jobs(bus, table, j->id, "GetJobBefore", "\twaiting for job"); } - r = table_print(table, NULL); + r = table_print(table); if (r < 0) return log_error_errno(r, "Failed to print the table: %m"); diff --git a/src/systemctl/systemctl-util.c b/src/systemctl/systemctl-util.c index ef7bce9e7f..b278f784ba 100644 --- a/src/systemctl/systemctl-util.c +++ b/src/systemctl/systemctl-util.c @@ -919,7 +919,7 @@ int output_table(Table *table) { if (OUTPUT_MODE_IS_JSON(arg_output)) r = table_print_json(table, NULL, output_mode_to_json_format_flags(arg_output) | SD_JSON_FORMAT_COLOR_AUTO); else - r = table_print(table, NULL); + r = table_print(table); if (r < 0) return table_log_print_error(r); diff --git a/src/test/test-format-table.c b/src/test/test-format-table.c index 4305f77224..677adaa9c9 100644 --- a/src/test/test-format-table.c +++ b/src/test/test-format-table.c @@ -890,7 +890,7 @@ TEST(table_ansi) { "FOO BAR BAZ KKK\n" "hallo knuerzredgreen noansi thisisgrey\n"); - ASSERT_OK(table_print(table, /* f= */ NULL)); + ASSERT_OK(table_print(table)); _cleanup_(sd_json_variant_unrefp) sd_json_variant *j = NULL, *jj = NULL; diff --git a/src/timedate/timedatectl.c b/src/timedate/timedatectl.c index cec4363aff..93a5dd6da3 100644 --- a/src/timedate/timedatectl.c +++ b/src/timedate/timedatectl.c @@ -163,7 +163,7 @@ static int print_status_info(const StatusInfo *i) { if (r < 0) return table_log_add_error(r); - r = table_print(table, NULL); + r = table_print(table); if (r < 0) return table_log_print_error(r); @@ -443,7 +443,7 @@ static int print_ntp_status_info(NTPStatusInfo *i) { if (r < 0) return table_log_add_error(r); - r = table_print(table, NULL); + r = table_print(table); if (r < 0) return table_log_print_error(r); @@ -452,7 +452,7 @@ static int print_ntp_status_info(NTPStatusInfo *i) { if (i->dest < i->origin || i->trans < i->recv || i->dest - i->origin < i->trans - i->recv) { log_error("Invalid NTP response"); - r = table_print(table, NULL); + r = table_print(table); if (r < 0) return table_log_print_error(r); @@ -536,7 +536,7 @@ static int print_ntp_status_info(NTPStatusInfo *i) { return table_log_add_error(r); } - r = table_print(table, NULL); + r = table_print(table); if (r < 0) return table_log_print_error(r); diff --git a/src/update-done/update-done.c b/src/update-done/update-done.c index b3c45c352b..f076f263a6 100644 --- a/src/update-done/update-done.c +++ b/src/update-done/update-done.c @@ -82,7 +82,7 @@ static int help(void) { ansi_normal(), ansi_underline(), ansi_normal()); - table_print(options, stdout); + table_print(options); printf("\nSee the %s for details.\n", link); return 0; diff --git a/src/validatefs/validatefs.c b/src/validatefs/validatefs.c index 9645fd187f..0608a14895 100644 --- a/src/validatefs/validatefs.c +++ b/src/validatefs/validatefs.c @@ -50,7 +50,7 @@ static int help(void) { program_invocation_short_name, ansi_highlight(), ansi_normal()); - table_print(options, stdout); + table_print(options); printf("\nSee the %s for details.\n", link); return 0; diff --git a/src/varlinkctl/varlinkctl.c b/src/varlinkctl/varlinkctl.c index c2cdd52b89..2d610db4e5 100644 --- a/src/varlinkctl/varlinkctl.c +++ b/src/varlinkctl/varlinkctl.c @@ -439,7 +439,7 @@ static int verb_info(int argc, char *argv[], uintptr_t _data, void *userdata) { if (r < 0) return table_log_add_error(r); - r = table_print(t, NULL); + r = table_print(t); if (r < 0) return table_log_print_error(r); } diff --git a/src/vpick/vpick-tool.c b/src/vpick/vpick-tool.c index c20994d115..1f3277da45 100644 --- a/src/vpick/vpick-tool.c +++ b/src/vpick/vpick-tool.c @@ -337,7 +337,7 @@ static int run(int argc, char *argv[]) { return table_log_add_error(r); } - r = table_print(t, stdout); + r = table_print(t); if (r < 0) return table_log_print_error(r); From 1008583c91474091eae5ebd3a55ddd86d41a6e85 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Zbigniew=20J=C4=99drzejewski-Szmek?= Date: Fri, 3 Apr 2026 11:49:30 +0200 Subject: [PATCH 06/11] tree-wide: print error if table_print() fails We generally want to print an error message if table_print() fails. Add a helper function for this and use it consistently. This does one of the three things depending on the call site: - a no-change reformatting of the code - change from a custom message to the generic one - addition of the error message where previously none was printed In the third case, the actual use impact is very small, since the table formatting is very unlikely to fail. But if it did, we would often return an error without any message whatsoever, which we never want to do. --- src/ac-power/ac-power.c | 4 +++- src/analyze/analyze-calendar.c | 2 +- src/analyze/analyze-image-policy.c | 2 +- src/analyze/analyze-inspect-elf.c | 4 ++-- src/analyze/analyze-timespan.c | 2 +- src/analyze/analyze-timestamp.c | 2 +- src/ask-password/ask-password.c | 4 +++- src/binfmt/binfmt.c | 4 +++- src/bless-boot/bless-boot.c | 8 ++++++-- src/cryptenroll/cryptenroll-list.c | 6 +----- src/detect-virt/detect-virt.c | 4 +++- src/dissect/dissect.c | 12 ++++++++---- src/factory-reset/factory-reset-tool.c | 8 ++++++-- src/hostname/hostnamectl.c | 14 +++++++------- src/id128/id128.c | 8 ++++++-- src/imds/imds-tool.c | 8 ++------ src/locale/localectl.c | 6 +----- src/login/loginctl.c | 12 ++++++------ src/network/networkctl-address-label.c | 6 +----- src/network/networkctl-list.c | 4 ++-- src/network/networkctl-lldp.c | 4 ++-- src/network/networkctl-status-link.c | 4 ++-- src/network/networkctl-status-system.c | 4 ++-- src/notify/notify.c | 4 +++- src/portable/portablectl.c | 4 ++-- src/report/report-basic-server.c | 3 +-- src/resolve/resolvectl.c | 16 ++++------------ src/shared/format-table.c | 9 +++++++++ src/shared/format-table.h | 1 + src/shared/libfido2-util.c | 6 ++---- src/shared/parse-argument.c | 6 +----- src/shared/pkcs11-util.c | 6 +----- src/shared/tpm2-util.c | 6 +----- src/systemctl/systemctl-list-jobs.c | 4 ++-- src/timedate/timedatectl.c | 22 +++++----------------- src/update-done/update-done.c | 4 +++- src/validatefs/validatefs.c | 4 +++- src/varlinkctl/varlinkctl.c | 14 +++++--------- src/vpick/vpick-tool.c | 4 ++-- 39 files changed, 115 insertions(+), 130 deletions(-) diff --git a/src/ac-power/ac-power.c b/src/ac-power/ac-power.c index b153194cdb..dad4384ada 100644 --- a/src/ac-power/ac-power.c +++ b/src/ac-power/ac-power.c @@ -37,7 +37,9 @@ static int help(void) { program_invocation_short_name, ansi_highlight(), ansi_normal()); - table_print(options); + r = table_print_or_warn(options); + if (r < 0) + return r; printf("\nSee the %s for details.\n", link); return 0; diff --git a/src/analyze/analyze-calendar.c b/src/analyze/analyze-calendar.c index ea99f3871e..c1427f25aa 100644 --- a/src/analyze/analyze-calendar.c +++ b/src/analyze/analyze-calendar.c @@ -119,7 +119,7 @@ static int test_calendar_one(usec_t n, const char *p) { n = next; } - return table_print(table); + return table_print_or_warn(table); } int verb_calendar(int argc, char *argv[], uintptr_t _data, void *userdata) { diff --git a/src/analyze/analyze-image-policy.c b/src/analyze/analyze-image-policy.c index 16e69414c1..220716878a 100644 --- a/src/analyze/analyze-image-policy.c +++ b/src/analyze/analyze-image-policy.c @@ -157,7 +157,7 @@ int verb_image_policy(int argc, char *argv[], uintptr_t _data, void *userdata) { putc('\n', stdout); - r = table_print(table); + r = table_print_or_warn(table); if (r < 0) return r; } diff --git a/src/analyze/analyze-inspect-elf.c b/src/analyze/analyze-inspect-elf.c index f4fcc3bd57..c664aea588 100644 --- a/src/analyze/analyze-inspect-elf.c +++ b/src/analyze/analyze-inspect-elf.c @@ -118,9 +118,9 @@ static int analyze_elf(char **filenames, sd_json_format_flags_t json_flags) { if (sd_json_format_enabled(json_flags)) sd_json_variant_dump(package_metadata, json_flags, stdout, NULL); else { - r = table_print(t); + r = table_print_or_warn(t); if (r < 0) - return table_log_print_error(r); + return r; } } diff --git a/src/analyze/analyze-timespan.c b/src/analyze/analyze-timespan.c index b6ca7bb4af..31a201c5f6 100644 --- a/src/analyze/analyze-timespan.c +++ b/src/analyze/analyze-timespan.c @@ -55,7 +55,7 @@ int verb_timespan(int argc, char *argv[], uintptr_t _data, void *userdata) { if (r < 0) return table_log_add_error(r); - r = table_print(table); + r = table_print_or_warn(table); if (r < 0) return r; diff --git a/src/analyze/analyze-timestamp.c b/src/analyze/analyze-timestamp.c index d998ca830a..5d4fa2f625 100644 --- a/src/analyze/analyze-timestamp.c +++ b/src/analyze/analyze-timestamp.c @@ -73,7 +73,7 @@ static int test_timestamp_one(const char *p) { if (r < 0) return table_log_add_error(r); - return table_print(table); + return table_print_or_warn(table); } int verb_timestamp(int argc, char *argv[], uintptr_t _data, void *userdata) { diff --git a/src/ask-password/ask-password.c b/src/ask-password/ask-password.c index 2ed2be8afe..4bd618b2a7 100644 --- a/src/ask-password/ask-password.c +++ b/src/ask-password/ask-password.c @@ -57,7 +57,9 @@ static int help(void) { program_invocation_short_name, ansi_highlight(), ansi_normal()); - table_print(options); + r = table_print_or_warn(options); + if (r < 0) + return r; printf("\nSee the %s for details.\n", link); return 0; diff --git a/src/binfmt/binfmt.c b/src/binfmt/binfmt.c index 06fff811bf..f8c2b55595 100644 --- a/src/binfmt/binfmt.c +++ b/src/binfmt/binfmt.c @@ -126,7 +126,9 @@ static int help(void) { program_invocation_short_name, ansi_highlight(), ansi_normal()); - table_print(options); + r = table_print_or_warn(options); + if (r < 0) + return r; printf("\nSee the %s for details.\n", link); return 0; diff --git a/src/bless-boot/bless-boot.c b/src/bless-boot/bless-boot.c index 67da021f29..86525f359a 100644 --- a/src/bless-boot/bless-boot.c +++ b/src/bless-boot/bless-boot.c @@ -58,10 +58,14 @@ static int help(void) { program_invocation_short_name, ansi_highlight(), ansi_normal()); - table_print(verbs); + r = table_print_or_warn(verbs); + if (r < 0) + return r; printf("\nOptions:\n"); - table_print(options); + r = table_print_or_warn(options); + if (r < 0) + return r; printf("\nSee the %s for details.\n", link); return 0; diff --git a/src/cryptenroll/cryptenroll-list.c b/src/cryptenroll/cryptenroll-list.c index bf9f2a130e..bca9f74c3a 100644 --- a/src/cryptenroll/cryptenroll-list.c +++ b/src/cryptenroll/cryptenroll-list.c @@ -125,9 +125,5 @@ int list_enrolled(struct crypt_device *cd) { return 0; } - r = table_print(t); - if (r < 0) - return log_error_errno(r, "Failed to show slot table: %m"); - - return 0; + return table_print_or_warn(t); } diff --git a/src/detect-virt/detect-virt.c b/src/detect-virt/detect-virt.c index a8b9739a92..3c5b3dd39c 100644 --- a/src/detect-virt/detect-virt.c +++ b/src/detect-virt/detect-virt.c @@ -41,7 +41,9 @@ static int help(void) { program_invocation_short_name, ansi_highlight(), ansi_normal()); - table_print(options); + r = table_print_or_warn(options); + if (r < 0) + return r; printf("\nSee the %s for details.\n", link); return 0; diff --git a/src/dissect/dissect.c b/src/dissect/dissect.c index 91ad3a4502..ceaedb262f 100644 --- a/src/dissect/dissect.c +++ b/src/dissect/dissect.c @@ -165,11 +165,15 @@ static int help(void) { ansi_underline(), ansi_normal()); - table_print(options); + r = table_print_or_warn(options); + if (r < 0) + return r; printf("\n%sCommands:%s\n", ansi_underline(), ansi_normal()); - table_print(commands); + r = table_print_or_warn(commands); + if (r < 0) + return r; printf("\nSee the %s for details.\n", link); return 0; @@ -1082,9 +1086,9 @@ static int action_dissect( if (!sd_json_format_enabled(arg_json_format_flags)) { table_set_header(t, arg_legend); - r = table_print(t); + r = table_print_or_warn(t); if (r < 0) - return table_log_print_error(r); + return r; } else { _cleanup_(sd_json_variant_unrefp) sd_json_variant *jt = NULL; diff --git a/src/factory-reset/factory-reset-tool.c b/src/factory-reset/factory-reset-tool.c index c09369e229..ec3f7e43c4 100644 --- a/src/factory-reset/factory-reset-tool.c +++ b/src/factory-reset/factory-reset-tool.c @@ -51,10 +51,14 @@ static int help(void) { program_invocation_short_name, ansi_highlight(), ansi_normal()); - table_print(verbs); + r = table_print_or_warn(verbs); + if (r < 0) + return r; printf("\nOptions:\n"); - table_print(options); + r = table_print_or_warn(options); + if (r < 0) + return r; printf("\nSee the %s for details.\n", link); return 0; diff --git a/src/hostname/hostnamectl.c b/src/hostname/hostnamectl.c index 52b31a5867..67e82c1e7c 100644 --- a/src/hostname/hostnamectl.c +++ b/src/hostname/hostnamectl.c @@ -375,11 +375,7 @@ static int print_status_info(StatusInfo *i) { } } - r = table_print(table); - if (r < 0) - return table_log_print_error(r); - - return 0; + return table_print_or_warn(table); } static int get_one_name(sd_bus *bus, const char* attr, char **ret) { @@ -748,10 +744,14 @@ static int help(void) { program_invocation_short_name, ansi_highlight(), ansi_normal()); - table_print(verbs); + r = table_print_or_warn(verbs); + if (r < 0) + return r; printf("\nOptions:\n"); - table_print(options); + r = table_print_or_warn(options); + if (r < 0) + return r; printf("\nSee the %s for details.\n", link); return 0; diff --git a/src/id128/id128.c b/src/id128/id128.c index eda117aec0..1616bfea65 100644 --- a/src/id128/id128.c +++ b/src/id128/id128.c @@ -216,10 +216,14 @@ static int help(void) { program_invocation_short_name, ansi_highlight(), ansi_normal()); - table_print(verbs); + r = table_print_or_warn(verbs); + if (r < 0) + return r; printf("\nOptions:\n"); - table_print(options); + r = table_print_or_warn(options); + if (r < 0) + return r; printf("\nSee the %s for details.\n", link); return 0; diff --git a/src/imds/imds-tool.c b/src/imds/imds-tool.c index 61fc82014e..ff5a9b317a 100644 --- a/src/imds/imds-tool.c +++ b/src/imds/imds-tool.c @@ -381,14 +381,10 @@ static int action_summary(sd_varlink *link) { if (table_isempty(table)) return log_error_errno(SYNTHETIC_ERRNO(EOPNOTSUPP), "No well-known IMDS data available."); - r = table_print(table); - if (r < 0) - return table_log_print_error(r); - - return 0; + return table_print_or_warn(table); } -static const char *detect_json_object(const char *text) { +static const char* detect_json_object(const char *text) { assert(text); /* Checks if the provided text looks like a JSON object. It checks if the first non-whitespace diff --git a/src/locale/localectl.c b/src/locale/localectl.c index b67a67e73b..e80cd96c86 100644 --- a/src/locale/localectl.c +++ b/src/locale/localectl.c @@ -144,11 +144,7 @@ static int print_status_info(StatusInfo *i) { return table_log_add_error(r); } - r = table_print(table); - if (r < 0) - return table_log_print_error(r); - - return 0; + return table_print_or_warn(table); } static int verb_show_status(int argc, char *argv[], uintptr_t _data, void *userdata) { diff --git a/src/login/loginctl.c b/src/login/loginctl.c index fa1c024481..cdffd79d8c 100644 --- a/src/login/loginctl.c +++ b/src/login/loginctl.c @@ -716,9 +716,9 @@ static int print_session_status_info(sd_bus *bus, const char *path) { /* We don't use the table to show the header, in order to make the width of the column stable. */ printf("%s%s - %s (" UID_FMT ")%s\n", ansi_highlight(), i.id, i.name, i.uid, ansi_normal()); - r = table_print(table); + r = table_print_or_warn(table); if (r < 0) - return table_log_print_error(r); + return r; if (i.scope) { show_unit_cgroup(bus, i.scope, i.leader, /* prefix= */ strrepa(" ", STRLEN("Display: "))); @@ -821,9 +821,9 @@ static int print_user_status_info(sd_bus *bus, const char *path) { printf("%s%s (" UID_FMT ")%s\n", ansi_highlight(), i.name, i.uid, ansi_normal()); - r = table_print(table); + r = table_print_or_warn(table); if (r < 0) - return table_log_print_error(r); + return r; if (i.slice) { show_unit_cgroup(bus, i.slice, /* leader= */ 0, /* prefix= */ strrepa(" ", STRLEN("Sessions: "))); @@ -896,9 +896,9 @@ static int print_seat_status_info(sd_bus *bus, const char *path) { printf("%s%s%s\n", ansi_highlight(), i.id, ansi_normal()); - r = table_print(table); + r = table_print_or_warn(table); if (r < 0) - return table_log_print_error(r); + return r; if (arg_transport == BUS_TRANSPORT_LOCAL) { unsigned c = MAX(LESS_BY(columns(), 21U), 10U); diff --git a/src/network/networkctl-address-label.c b/src/network/networkctl-address-label.c index 04b6d4d236..c1e2cc0920 100644 --- a/src/network/networkctl-address-label.c +++ b/src/network/networkctl-address-label.c @@ -82,11 +82,7 @@ static int dump_address_labels(sd_netlink *rtnl) { return table_log_add_error(r); } - r = table_print(table); - if (r < 0) - return table_log_print_error(r); - - return 0; + return table_print_or_warn(table); } int verb_list_address_labels(int argc, char *argv[], uintptr_t _data, void *userdata) { diff --git a/src/network/networkctl-list.c b/src/network/networkctl-list.c index c30be4a1de..6f3379e788 100644 --- a/src/network/networkctl-list.c +++ b/src/network/networkctl-list.c @@ -79,9 +79,9 @@ int verb_list_links(int argc, char *argv[], uintptr_t _data, void *userdata) { return table_log_add_error(r); } - r = table_print(table); + r = table_print_or_warn(table); if (r < 0) - return table_log_print_error(r); + return r; if (arg_legend) printf("\n%i links listed.\n", c); diff --git a/src/network/networkctl-lldp.c b/src/network/networkctl-lldp.c index c91d8fea00..009f70e4d7 100644 --- a/src/network/networkctl-lldp.c +++ b/src/network/networkctl-lldp.c @@ -303,9 +303,9 @@ int verb_link_lldp_status(int argc, char *argv[], uintptr_t _data, void *userdat } } - r = table_print(table); + r = table_print_or_warn(table); if (r < 0) - return table_log_print_error(r); + return r; if (arg_legend) { lldp_capabilities_legend(all); diff --git a/src/network/networkctl-status-link.c b/src/network/networkctl-status-link.c index 9cbf3efb33..15c9c46336 100644 --- a/src/network/networkctl-status-link.c +++ b/src/network/networkctl-status-link.c @@ -900,9 +900,9 @@ static int link_status_one( on_color_operational, glyph(GLYPH_BLACK_CIRCLE), off_color_operational, info->ifindex, info->name); - r = table_print(table); + r = table_print_or_warn(table); if (r < 0) - return table_log_print_error(r); + return r; return show_logs(info->ifindex, info->name); } diff --git a/src/network/networkctl-status-system.c b/src/network/networkctl-status-system.c index ce403d6062..a03004b882 100644 --- a/src/network/networkctl-status-system.c +++ b/src/network/networkctl-status-system.c @@ -126,9 +126,9 @@ int system_status(sd_netlink *rtnl, sd_hwdb *hwdb) { on_color_operational, glyph(GLYPH_BLACK_CIRCLE), off_color_operational, strna(netifs_joined)); - r = table_print(table); + r = table_print_or_warn(table); if (r < 0) - return table_log_print_error(r); + return r; return show_logs(0, NULL); } diff --git a/src/notify/notify.c b/src/notify/notify.c index e425a125ac..2ac0129bae 100644 --- a/src/notify/notify.c +++ b/src/notify/notify.c @@ -77,7 +77,9 @@ static int help(void) { ansi_highlight(), ansi_normal()); - table_print(options); + r = table_print_or_warn(options); + if (r < 0) + return r; printf("\nSee the %s for details.\n", link); return 0; diff --git a/src/portable/portablectl.c b/src/portable/portablectl.c index f5ce3db9c4..7c2cbf3e52 100644 --- a/src/portable/portablectl.c +++ b/src/portable/portablectl.c @@ -1075,9 +1075,9 @@ static int verb_list_images(int argc, char *argv[], uintptr_t _data, void *userd table_set_header(table, arg_legend); - r = table_print(table); + r = table_print_or_warn(table); if (r < 0) - return table_log_print_error(r); + return r; } if (arg_legend) { diff --git a/src/report/report-basic-server.c b/src/report/report-basic-server.c index dc482a37ff..ea82dbd42f 100644 --- a/src/report/report-basic-server.c +++ b/src/report/report-basic-server.c @@ -47,9 +47,8 @@ static int help(void) { ansi_normal(), ansi_underline(), ansi_normal()); - table_print(options); - return 0; + return table_print_or_warn(options); } static int parse_argv(int argc, char *argv[]) { diff --git a/src/resolve/resolvectl.c b/src/resolve/resolvectl.c index fe2b676c7c..8cc9e73f93 100644 --- a/src/resolve/resolvectl.c +++ b/src/resolve/resolvectl.c @@ -1321,11 +1321,7 @@ static int verb_show_statistics(int argc, char *argv[], uintptr_t _data, void *u if (r < 0) return table_log_add_error(r); - r = table_print(table); - if (r < 0) - return table_log_print_error(r); - - return 0; + return table_print_or_warn(table); } static int verb_reset_statistics(int argc, char *argv[], uintptr_t _data, void *userdata) { @@ -1890,9 +1886,9 @@ static int print_configuration(DNSConfiguration *configuration, StatusMode mode, return table_log_add_error(r); } - r = table_print(table); + r = table_print_or_warn(table); if (r < 0) - return table_log_print_error(r); + return r; if (empty_line) *empty_line = true; @@ -3097,11 +3093,7 @@ static int dump_server_state(sd_json_variant *server) { if (r < 0) return table_log_add_error(r); - r = table_print(table); - if (r < 0) - return table_log_print_error(r); - - return 0; + return table_print_or_warn(table); } static int verb_show_server_state(int argc, char *argv[], uintptr_t _data, void *userdata) { diff --git a/src/shared/format-table.c b/src/shared/format-table.c index 718d5c09b8..8c8bcb1bdc 100644 --- a/src/shared/format-table.c +++ b/src/shared/format-table.c @@ -2643,6 +2643,15 @@ int table_print_full(Table *t, FILE *f, bool flush) { return fflush_and_check(f); } +int table_print_or_warn(Table *t) { + int r; + + r = table_print(t); + if (r < 0) + return table_log_print_error(r); + return 0; +} + int table_format(Table *t, char **ret) { _cleanup_(memstream_done) MemStream m = {}; FILE *f; diff --git a/src/shared/format-table.h b/src/shared/format-table.h index ee4007394e..0f52f80293 100644 --- a/src/shared/format-table.h +++ b/src/shared/format-table.h @@ -151,6 +151,7 @@ int table_print_full(Table *t, FILE *f, bool flush); static inline int table_print(Table *t) { return table_print_full(t, /* f= */ NULL, /* flush= */ false); } +int table_print_or_warn(Table *t); int table_format(Table *t, char **ret); diff --git a/src/shared/libfido2-util.c b/src/shared/libfido2-util.c index 7e50d0dd2d..6224ad4b1d 100644 --- a/src/shared/libfido2-util.c +++ b/src/shared/libfido2-util.c @@ -1241,11 +1241,9 @@ int fido2_list_devices(void) { } } - r = table_print(t); - if (r < 0) { - log_error_errno(r, "Failed to show device table: %m"); + r = table_print_or_warn(t); + if (r < 0) goto finish; - } if (!table_isempty(t)) printf("\n" diff --git a/src/shared/parse-argument.c b/src/shared/parse-argument.c index 6ac42c4d88..e85c78a476 100644 --- a/src/shared/parse-argument.c +++ b/src/shared/parse-argument.c @@ -128,11 +128,7 @@ int parse_signal_argument(const char *s, int *ret) { return table_log_add_error(r); } - r = table_print(table); - if (r < 0) - return table_log_print_error(r); - - return 0; + return table_print_or_warn(table); } r = signal_from_string(s); diff --git a/src/shared/pkcs11-util.c b/src/shared/pkcs11-util.c index 2d8ce29b58..3144d0b3da 100644 --- a/src/shared/pkcs11-util.c +++ b/src/shared/pkcs11-util.c @@ -1816,11 +1816,7 @@ int pkcs11_list_tokens(void) { return 0; } - r = table_print(t); - if (r < 0) - return log_error_errno(r, "Failed to show device table: %m"); - - return 0; + return table_print_or_warn(t); #else return log_error_errno(SYNTHETIC_ERRNO(EOPNOTSUPP), "PKCS#11 tokens not supported on this build."); diff --git a/src/shared/tpm2-util.c b/src/shared/tpm2-util.c index a1f05d6397..78ff47afe8 100644 --- a/src/shared/tpm2-util.c +++ b/src/shared/tpm2-util.c @@ -6584,11 +6584,7 @@ int tpm2_list_devices(bool legend, bool quiet) { return 0; } - r = table_print(t); - if (r < 0) - return log_error_errno(r, "Failed to show device table: %m"); - - return 0; + return table_print_or_warn(t); #else return log_error_errno(SYNTHETIC_ERRNO(EOPNOTSUPP), "TPM2 not supported on this build."); diff --git a/src/systemctl/systemctl-list-jobs.c b/src/systemctl/systemctl-list-jobs.c index bf3f5ed866..e5ad25234f 100644 --- a/src/systemctl/systemctl-list-jobs.c +++ b/src/systemctl/systemctl-list-jobs.c @@ -115,9 +115,9 @@ static int output_jobs_list(sd_bus *bus, const struct job_info* jobs, unsigned n output_waiting_jobs(bus, table, j->id, "GetJobBefore", "\twaiting for job"); } - r = table_print(table); + r = table_print_or_warn(table); if (r < 0) - return log_error_errno(r, "Failed to print the table: %m"); + return r; if (arg_legend != 0) { on = ansi_highlight(); diff --git a/src/timedate/timedatectl.c b/src/timedate/timedatectl.c index 93a5dd6da3..d02cb80bb1 100644 --- a/src/timedate/timedatectl.c +++ b/src/timedate/timedatectl.c @@ -163,9 +163,9 @@ static int print_status_info(const StatusInfo *i) { if (r < 0) return table_log_add_error(r); - r = table_print(table); + r = table_print_or_warn(table); if (r < 0) - return table_log_print_error(r); + return r; if (i->rtc_local) { fflush(stdout); @@ -443,20 +443,12 @@ static int print_ntp_status_info(NTPStatusInfo *i) { if (r < 0) return table_log_add_error(r); - r = table_print(table); - if (r < 0) - return table_log_print_error(r); - - return 0; + return table_print_or_warn(table); } if (i->dest < i->origin || i->trans < i->recv || i->dest - i->origin < i->trans - i->recv) { log_error("Invalid NTP response"); - r = table_print(table); - if (r < 0) - return table_log_print_error(r); - - return 0; + return table_print_or_warn(table); } delay = (i->dest - i->origin) - (i->trans - i->recv); @@ -536,11 +528,7 @@ static int print_ntp_status_info(NTPStatusInfo *i) { return table_log_add_error(r); } - r = table_print(table); - if (r < 0) - return table_log_print_error(r); - - return 0; + return table_print_or_warn(table); } static int map_server_address(sd_bus *bus, const char *member, sd_bus_message *m, sd_bus_error *error, void *userdata) { diff --git a/src/update-done/update-done.c b/src/update-done/update-done.c index f076f263a6..bc678b8f41 100644 --- a/src/update-done/update-done.c +++ b/src/update-done/update-done.c @@ -82,7 +82,9 @@ static int help(void) { ansi_normal(), ansi_underline(), ansi_normal()); - table_print(options); + r = table_print_or_warn(options); + if (r < 0) + return r; printf("\nSee the %s for details.\n", link); return 0; diff --git a/src/validatefs/validatefs.c b/src/validatefs/validatefs.c index 0608a14895..1106eaf1fe 100644 --- a/src/validatefs/validatefs.c +++ b/src/validatefs/validatefs.c @@ -50,7 +50,9 @@ static int help(void) { program_invocation_short_name, ansi_highlight(), ansi_normal()); - table_print(options); + r = table_print_or_warn(options); + if (r < 0) + return r; printf("\nSee the %s for details.\n", link); return 0; diff --git a/src/varlinkctl/varlinkctl.c b/src/varlinkctl/varlinkctl.c index 2d610db4e5..4124d0d570 100644 --- a/src/varlinkctl/varlinkctl.c +++ b/src/varlinkctl/varlinkctl.c @@ -416,6 +416,8 @@ static int verb_info(int argc, char *argv[], uintptr_t _data, void *userdata) { if (streq_ptr(argv[0], "list-interfaces")) { STRV_FOREACH(i, data.interfaces) puts(*i); + + return 0; } else { _cleanup_(table_unrefp) Table *t = NULL; @@ -439,20 +441,14 @@ static int verb_info(int argc, char *argv[], uintptr_t _data, void *userdata) { if (r < 0) return table_log_add_error(r); - r = table_print(t); - if (r < 0) - return table_log_print_error(r); + return table_print_or_warn(t); } } else { - sd_json_variant *v; - - v = streq_ptr(argv[0], "list-interfaces") ? + sd_json_variant *v = streq_ptr(argv[0], "list-interfaces") ? sd_json_variant_by_key(reply, "interfaces") : reply; - sd_json_variant_dump(v, arg_json_format_flags, stdout, NULL); + return sd_json_variant_dump(v, arg_json_format_flags, stdout, NULL); } - - return 0; } static size_t break_columns(void) { diff --git a/src/vpick/vpick-tool.c b/src/vpick/vpick-tool.c index 1f3277da45..57df857c6a 100644 --- a/src/vpick/vpick-tool.c +++ b/src/vpick/vpick-tool.c @@ -337,9 +337,9 @@ static int run(int argc, char *argv[]) { return table_log_add_error(r); } - r = table_print(t); + r = table_print_or_warn(t); if (r < 0) - return table_log_print_error(r); + return r; break; } From 4419840ad1d86cf388d5fd8e9b2710faff02c284 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Zbigniew=20J=C4=99drzejewski-Szmek?= Date: Fri, 3 Apr 2026 11:56:04 +0200 Subject: [PATCH 07/11] analyze: consistently print error if table formatting fails We don't want to return an error without printing something. So for things which don't matter, explicitly suppress the error with (void). In other cases, add the standard message. --- src/analyze/analyze-blame.c | 16 +++++----------- src/analyze/analyze-calendar.c | 8 ++------ src/analyze/analyze-timespan.c | 8 ++------ src/analyze/analyze-timestamp.c | 10 +++------- 4 files changed, 12 insertions(+), 30 deletions(-) diff --git a/src/analyze/analyze-blame.c b/src/analyze/analyze-blame.c index 24b29a26a1..d400380ade 100644 --- a/src/analyze/analyze-blame.c +++ b/src/analyze/analyze-blame.c @@ -29,26 +29,20 @@ int verb_blame(int argc, char *argv[], uintptr_t _data, void *userdata) { return log_oom(); assert_se(cell = table_get_cell(table, 0, 0)); - r = table_set_ellipsize_percent(table, cell, 100); - if (r < 0) - return r; + (void) table_set_ellipsize_percent(table, cell, 100); - r = table_set_align_percent(table, cell, 100); - if (r < 0) - return r; + (void) table_set_align_percent(table, cell, 100); assert_se(cell = table_get_cell(table, 0, 1)); - r = table_set_ellipsize_percent(table, cell, 100); - if (r < 0) - return r; + (void) table_set_ellipsize_percent(table, cell, 100); r = table_set_sort(table, (size_t) 0); if (r < 0) - return r; + return table_log_sort_error(r); r = table_set_reverse(table, 0, true); if (r < 0) - return r; + return table_log_sort_error(r); for (UnitTimes *u = times; u->has_data; u++) { if (u->time <= 0) diff --git a/src/analyze/analyze-calendar.c b/src/analyze/analyze-calendar.c index c1427f25aa..a9fb1e897e 100644 --- a/src/analyze/analyze-calendar.c +++ b/src/analyze/analyze-calendar.c @@ -35,14 +35,10 @@ static int test_calendar_one(usec_t n, const char *p) { return log_oom(); assert_se(cell = table_get_cell(table, 0, 0)); - r = table_set_ellipsize_percent(table, cell, 100); - if (r < 0) - return r; + (void) table_set_ellipsize_percent(table, cell, 100); assert_se(cell = table_get_cell(table, 0, 1)); - r = table_set_ellipsize_percent(table, cell, 100); - if (r < 0) - return r; + (void) table_set_ellipsize_percent(table, cell, 100); if (!streq(t, p)) { r = table_add_many(table, diff --git a/src/analyze/analyze-timespan.c b/src/analyze/analyze-timespan.c index 31a201c5f6..de78736e2c 100644 --- a/src/analyze/analyze-timespan.c +++ b/src/analyze/analyze-timespan.c @@ -28,14 +28,10 @@ int verb_timespan(int argc, char *argv[], uintptr_t _data, void *userdata) { return log_oom(); assert_se(cell = table_get_cell(table, 0, 0)); - r = table_set_ellipsize_percent(table, cell, 100); - if (r < 0) - return r; + (void) table_set_ellipsize_percent(table, cell, 100); assert_se(cell = table_get_cell(table, 0, 1)); - r = table_set_ellipsize_percent(table, cell, 100); - if (r < 0) - return r; + (void) table_set_ellipsize_percent(table, cell, 100); r = table_add_many(table, TABLE_FIELD, "Original", diff --git a/src/analyze/analyze-timestamp.c b/src/analyze/analyze-timestamp.c index 5d4fa2f625..50cda12d71 100644 --- a/src/analyze/analyze-timestamp.c +++ b/src/analyze/analyze-timestamp.c @@ -27,14 +27,10 @@ static int test_timestamp_one(const char *p) { return log_oom(); assert_se(cell = table_get_cell(table, 0, 0)); - r = table_set_ellipsize_percent(table, cell, 100); - if (r < 0) - return r; + (void) table_set_ellipsize_percent(table, cell, 100); assert_se(cell = table_get_cell(table, 0, 1)); - r = table_set_ellipsize_percent(table, cell, 100); - if (r < 0) - return r; + (void) table_set_ellipsize_percent(table, cell, 100); r = table_add_many(table, TABLE_FIELD, "Original form", @@ -65,7 +61,7 @@ static int test_timestamp_one(const char *p) { usec / USEC_PER_SEC, usec % USEC_PER_SEC); if (r < 0) - return r; + return table_log_add_error(r); r = table_add_many(table, TABLE_FIELD, "From now", From fec559d034ed32929771c80922fd2c2249edba15 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Zbigniew=20J=C4=99drzejewski-Szmek?= Date: Thu, 2 Apr 2026 22:20:48 +0200 Subject: [PATCH 08/11] meson: detect and use __attribute__((no_reorder)) In some builds (package builds, so with optimization and lto, but I haven't been able to pin down the exact combination on options that matters), we end up with items in the verbs array reordered. The order matters (because of groups, but also because we have some specific order for display), so this reordering is something that we don't want. From what I was able to read, the compiler + linker generally keep the order within a single translation unit, but this is more of a convention and implementation choice than a guarantee. Add this attribute [1]. It seems to have the desired effect in CI. [1] https://gcc.gnu.org/onlinedocs/gcc-15.2.0/gcc/Common-Function-Attributes.html#index-no_005freorder-function-attribute --- meson.build | 9 +++++++++ src/fundamental/macro-fundamental.h | 6 ++++++ src/shared/options.h | 1 + src/shared/verbs.h | 1 + 4 files changed, 17 insertions(+) diff --git a/meson.build b/meson.build index c9e96b2591..0f8f0b5375 100644 --- a/meson.build +++ b/meson.build @@ -549,6 +549,15 @@ foreach attr : possible_c_attributes conf.set10('HAVE_ATTRIBUTE_' + attr.to_upper(), have) endforeach +# TODO: drop this manual check when meson learns about this attribute +possible_c_attributes += ['no_reorder'] + +have = cc.compiles( + '__attribute__((__no_reorder__)) int x;', + args : '-Werror=attributes', + name : '__attribute__((__no_reorder__))') +conf.set10('HAVE_ATTRIBUTE_NO_REORDER', have) + ##################################################################### # compilation result tests diff --git a/src/fundamental/macro-fundamental.h b/src/fundamental/macro-fundamental.h index 5690859cf4..f595be2cc5 100644 --- a/src/fundamental/macro-fundamental.h +++ b/src/fundamental/macro-fundamental.h @@ -116,6 +116,12 @@ # define _retain_ #endif +#if HAVE_ATTRIBUTE_NO_REORDER +# define _no_reorder_ __attribute__((__no_reorder__)) +#else +# define _no_reorder_ +#endif + #if __GNUC__ >= 15 # define _nonnull_if_nonzero_(p, n) __attribute__((nonnull_if_nonzero(p, n))) #else diff --git a/src/shared/options.h b/src/shared/options.h index afa17d9e30..c1ffa595ce 100644 --- a/src/shared/options.h +++ b/src/shared/options.h @@ -25,6 +25,7 @@ typedef struct Option { _alignptr_ \ _used_ \ _retain_ \ + _no_reorder_ \ _variable_no_sanitize_address_ \ static const Option CONCATENATE(option, counter) = { \ .id = 0x100 + counter, \ diff --git a/src/shared/verbs.h b/src/shared/verbs.h index cd18359cd7..51a9e3a525 100644 --- a/src/shared/verbs.h +++ b/src/shared/verbs.h @@ -26,6 +26,7 @@ typedef struct { _alignptr_ \ _used_ \ _retain_ \ + _no_reorder_ \ _variable_no_sanitize_address_ \ static const Verb CONCATENATE(verb_data_, __COUNTER__) = { \ .verb = v, \ From 190385b55dea5505bfd7fcd3dff3f48b38ee34ab Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Zbigniew=20J=C4=99drzejewski-Szmek?= Date: Tue, 24 Mar 2026 10:35:30 +0100 Subject: [PATCH 09/11] vpick: use the new option parser -h/--help and --version are moved from a standalone section into the Output group, because they look misplaced without a header and having a new "Options" group with the verb-like entries also wasn't appealing. Output is changed a bit to avoid repeating "rather than path": - -p --print=filename Print selected filename rather than path - -p --print=version Print selected version rather than path - -p --print=type Print selected inode type rather than path - -p --print=arch Print selected architecture rather than path - -p --print=tries Print selected tries left/tries done rather than path - -p --print=all Print all of the above - --resolve=yes Canonicalize the result path + -h --help Show this help + --version Show package version + -p --print=WHAT Print selected WHAT rather than path + --print=filename ... print selected filename + --print=version ... print selected version + --print=type ... print selected inode + --print=arch ... print selected architecture + --print=tries ... print selected tries left/tries done + --print=all ... print all of the above + --resolve=BOOL Canonicalize the result path Co-developed-by: Claude --- src/vpick/vpick-tool.c | 184 ++++++++++++++++++++--------------------- 1 file changed, 92 insertions(+), 92 deletions(-) diff --git a/src/vpick/vpick-tool.c b/src/vpick/vpick-tool.c index 57df857c6a..595ecae688 100644 --- a/src/vpick/vpick-tool.c +++ b/src/vpick/vpick-tool.c @@ -1,7 +1,6 @@ /* SPDX-License-Identifier: LGPL-2.1-or-later */ #include -#include #include "alloc-util.h" #include "architecture.h" @@ -9,12 +8,14 @@ #include "format-table.h" #include "log.h" #include "main-func.h" +#include "options.h" #include "parse-util.h" #include "path-util.h" #include "pretty-print.h" #include "stat-util.h" #include "string-table.h" #include "string-util.h" +#include "strv.h" #include "vpick.h" typedef enum { @@ -55,167 +56,165 @@ DEFINE_PRIVATE_STRING_TABLE_LOOKUP_FROM_STRING(print, Print); static int help(void) { _cleanup_free_ char *link = NULL; + _cleanup_(table_unrefp) Table *lookup_keys = NULL, *output = NULL; int r; r = terminal_urlify_man("systemd-vpick", "1", &link); if (r < 0) return log_oom(); - printf("%1$s [OPTIONS...] PATH...\n" - "\n%5$sPick entry from versioned directory.%6$s\n\n" - " -h --help Show this help\n" - " --version Show package version\n" - "\n%3$sLookup Keys:%4$s\n" - " -B --basename=BASENAME\n" - " Look for specified basename\n" - " -V VERSION Look for specified version\n" - " -A ARCH Look for specified architecture\n" - " -S --suffix=SUFFIX Look for specified suffix\n" - " -t --type=TYPE Look for specified inode type\n" - "\n%3$sOutput:%4$s\n" - " -p --print=filename Print selected filename rather than path\n" - " -p --print=version Print selected version rather than path\n" - " -p --print=type Print selected inode type rather than path\n" - " -p --print=arch Print selected architecture rather than path\n" - " -p --print=tries Print selected tries left/tries done rather than path\n" - " -p --print=all Print all of the above\n" - " --resolve=yes Canonicalize the result path\n" - "\nSee the %2$s for details.\n", - program_invocation_short_name, - link, - ansi_underline(), ansi_normal(), - ansi_highlight(), ansi_normal()); + r = option_parser_get_help_table(&lookup_keys); + if (r < 0) + return r; + r = option_parser_get_help_table_group("Output", &output); + if (r < 0) + return r; + + (void) table_sync_column_widths(0, lookup_keys, output); + + printf("%s [OPTIONS...] PATH...\n" + "\n%sPick entry from versioned directory.%s\n", + program_invocation_short_name, + ansi_highlight(), + ansi_normal()); + + printf("\n%sLookup Keys:%s\n", ansi_underline(), ansi_normal()); + r = table_print_or_warn(lookup_keys); + if (r < 0) + return r; + + printf("\n%sOutput:%s\n", ansi_underline(), ansi_normal()); + r = table_print_or_warn(output); + if (r < 0) + return r; + + printf("\nSee the %s for details.\n", link); return 0; } -static int parse_argv(int argc, char *argv[]) { - - enum { - ARG_VERSION = 0x100, - ARG_RESOLVE, - }; - - static const struct option options[] = { - { "help", no_argument, NULL, 'h' }, - { "version", no_argument, NULL, ARG_VERSION }, - { "basename", required_argument, NULL, 'B' }, - { "suffix", required_argument, NULL, 'S' }, - { "type", required_argument, NULL, 't' }, - { "print", required_argument, NULL, 'p' }, - { "resolve", required_argument, NULL, ARG_RESOLVE }, - {} - }; - - int c, r; +static int parse_argv(int argc, char *argv[], char ***ret_args) { + int r; assert(argc >= 0); assert(argv); - while ((c = getopt_long(argc, argv, "hB:V:A:S:t:p:", options, NULL)) >= 0) { + OptionParser state = { argc, argv }; + const char *arg; + FOREACH_OPTION(&state, c, &arg, /* on_error= */ return c) switch (c) { - case 'h': - return help(); + OPTION('B', "basename", "BASENAME", "Look for specified basename"): + if (!filename_part_is_valid(arg)) + return log_error_errno(SYNTHETIC_ERRNO(EINVAL), "Invalid basename string: %s", arg); - case ARG_VERSION: - return version(); - - case 'B': - if (!filename_part_is_valid(optarg)) - return log_error_errno(SYNTHETIC_ERRNO(EINVAL), "Invalid basename string: %s", optarg); - - r = free_and_strdup_warn(&arg_filter_basename, optarg); + r = free_and_strdup_warn(&arg_filter_basename, arg); if (r < 0) return r; break; - case 'V': - if (!version_is_valid(optarg)) - return log_error_errno(SYNTHETIC_ERRNO(EINVAL), "Invalid version string: %s", optarg); + OPTION_SHORT('V', "VERSION", "Look for specified version"): + if (!version_is_valid(arg)) + return log_error_errno(SYNTHETIC_ERRNO(EINVAL), "Invalid version string: %s", arg); - r = free_and_strdup_warn(&arg_filter_version, optarg); + r = free_and_strdup_warn(&arg_filter_version, arg); if (r < 0) return r; break; - case 'A': - if (streq(optarg, "native")) + OPTION_SHORT('A', "ARCH", "Look for specified architecture"): + if (streq(arg, "native")) arg_filter_architecture = native_architecture(); - else if (streq(optarg, "secondary")) { + else if (streq(arg, "secondary")) { #ifdef ARCHITECTURE_SECONDARY arg_filter_architecture = ARCHITECTURE_SECONDARY; #else return log_error_errno(SYNTHETIC_ERRNO(EOPNOTSUPP), "Local architecture has no secondary architecture."); #endif - } else if (streq(optarg, "uname")) + } else if (streq(arg, "uname")) arg_filter_architecture = uname_architecture(); - else if (streq(optarg, "auto")) + else if (streq(arg, "auto")) arg_filter_architecture = _ARCHITECTURE_INVALID; else { - arg_filter_architecture = architecture_from_string(optarg); + arg_filter_architecture = architecture_from_string(arg); if (arg_filter_architecture < 0) - return log_error_errno(SYNTHETIC_ERRNO(EINVAL), "Unknown architecture: %s", optarg); + return log_error_errno(SYNTHETIC_ERRNO(EINVAL), "Unknown architecture: %s", arg); } break; - case 'S': - if (!filename_part_is_valid(optarg)) - return log_error_errno(SYNTHETIC_ERRNO(EINVAL), "Invalid suffix string: %s", optarg); + OPTION('S', "suffix", "SUFFIX", "Look for specified suffix"): + if (!filename_part_is_valid(arg)) + return log_error_errno(SYNTHETIC_ERRNO(EINVAL), "Invalid suffix string: %s", arg); - r = free_and_strdup_warn(&arg_filter_suffix, optarg); + r = free_and_strdup_warn(&arg_filter_suffix, arg); if (r < 0) return r; break; - case 't': - if (isempty(optarg)) + OPTION('t', "type", "TYPE", "Look for specified inode type"): + if (isempty(arg)) arg_filter_type_mask = 0; else { mode_t m; - m = inode_type_from_string(optarg); + m = inode_type_from_string(arg); if (m == MODE_INVALID) - return log_error_errno(m, "Unknown inode type: %s", optarg); + return log_error_errno(m, "Unknown inode type: %s", arg); arg_filter_type_mask |= UINT32_C(1) << IFTODT(m); } break; - case 'p': - if (streq(optarg, "arch")) /* accept abbreviation too */ + OPTION_GROUP("Output"): {} + + OPTION_COMMON_HELP: + return help(); + + OPTION_COMMON_VERSION: + return version(); + + OPTION('p', "print", "WHAT", + "Print selected WHAT rather than path"): {} + OPTION_LONG_FLAGS(OPTION_HELP_ENTRY, "print", "filename", + "... print selected filename"): {} + OPTION_LONG_FLAGS(OPTION_HELP_ENTRY, "print", "version", + "... print selected version"): {} + OPTION_LONG_FLAGS(OPTION_HELP_ENTRY, "print", "type", + "... print selected inode type"): {} + OPTION_LONG_FLAGS(OPTION_HELP_ENTRY, "print", "arch", + "... print selected architecture"): {} + OPTION_LONG_FLAGS(OPTION_HELP_ENTRY, "print", "tries", + "... print selected tries left/tries done"): {} + OPTION_LONG_FLAGS(OPTION_HELP_ENTRY, "print", "all", + "... print all of the above"): + + if (streq(arg, "arch")) /* accept abbreviation too */ arg_print = PRINT_ARCHITECTURE; else - arg_print = print_from_string(optarg); + arg_print = print_from_string(arg); if (arg_print < 0) - return log_error_errno(arg_print, "Unknown --print= argument: %s", optarg); + return log_error_errno(arg_print, "Unknown --print= argument: %s", arg); break; - case ARG_RESOLVE: - r = parse_boolean(optarg); + OPTION_LONG("resolve", "BOOL", "Canonicalize the result path"): + r = parse_boolean(arg); if (r < 0) return log_error_errno(r, "Failed to parse --resolve= value: %m"); SET_FLAG(arg_flags, PICK_RESOLVE, r); break; - - case '?': - return -EINVAL; - - default: - assert_not_reached(); } - } if (arg_print < 0) arg_print = PRINT_PATH; + *ret_args = option_parser_get_args(&state); return 1; } @@ -224,18 +223,19 @@ static int run(int argc, char *argv[]) { log_setup(); - r = parse_argv(argc, argv); + char **args = NULL; + r = parse_argv(argc, argv, &args); if (r <= 0) return r; - if (optind >= argc) + if (strv_isempty(args)) return log_error_errno(SYNTHETIC_ERRNO(EINVAL), "Path to resolve must be specified."); - for (int i = optind; i < argc; i++) { + STRV_FOREACH(i, args) { _cleanup_free_ char *p = NULL; - r = path_make_absolute_cwd(argv[i], &p); + r = path_make_absolute_cwd(*i, &p); if (r < 0) - return log_error_errno(r, "Failed to make path '%s' absolute: %m", argv[i]); + return log_error_errno(r, "Failed to make path '%s' absolute: %m", *i); _cleanup_(pick_result_done) PickResult result = PICK_RESULT_NULL; r = path_pick(/* toplevel_path= */ NULL, From 94db1a961262fa577fd7b1f4c96eeaced748b9d9 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Zbigniew=20J=C4=99drzejewski-Szmek?= Date: Tue, 24 Mar 2026 10:06:14 +0100 Subject: [PATCH 10/11] timedatectl: use the new option and verb wrappers --help is identical, except for alignment of second column and changed strings in the standarized options. Co-developed-by: Claude --- src/timedate/timedatectl.c | 244 ++++++++++++++++--------------------- 1 file changed, 106 insertions(+), 138 deletions(-) diff --git a/src/timedate/timedatectl.c b/src/timedate/timedatectl.c index d02cb80bb1..75c142e84e 100644 --- a/src/timedate/timedatectl.c +++ b/src/timedate/timedatectl.c @@ -1,6 +1,5 @@ /* SPDX-License-Identifier: LGPL-2.1-or-later */ -#include #include #include #include @@ -20,6 +19,7 @@ #include "in-addr-util.h" #include "log.h" #include "main-func.h" +#include "options.h" #include "pager.h" #include "parse-argument.h" #include "parse-util.h" @@ -180,7 +180,8 @@ static int print_status_info(const StatusInfo *i) { return 0; } -static int verb_show_status(int argc, char *argv[], uintptr_t _data, void *userdata) { +VERB(verb_status, "status", NULL, VERB_ANY, 1, VERB_DEFAULT, "Show current time settings"); +static int verb_status(int argc, char *argv[], uintptr_t _data, void *userdata) { StatusInfo info = {}; static const struct bus_properties_map map[] = { { "Timezone", "s", NULL, offsetof(StatusInfo, timezone) }, @@ -212,7 +213,8 @@ static int verb_show_status(int argc, char *argv[], uintptr_t _data, void *userd return print_status_info(&info); } -static int verb_show_properties(int argc, char *argv[], uintptr_t _data, void *userdata) { +VERB_NOARG(verb_show, "show", "Show properties of systemd-timedated"); +static int verb_show(int argc, char *argv[], uintptr_t _data, void *userdata) { sd_bus *bus = ASSERT_PTR(userdata); int r; @@ -229,6 +231,7 @@ static int verb_show_properties(int argc, char *argv[], uintptr_t _data, void *u return 0; } +VERB(verb_set_time, "set-time", "TIME", 2, 2, 0, "Set system time"); static int verb_set_time(int argc, char *argv[], uintptr_t _data, void *userdata) { _cleanup_(sd_bus_error_free) sd_bus_error error = SD_BUS_ERROR_NULL; sd_bus *bus = userdata; @@ -254,6 +257,7 @@ static int verb_set_time(int argc, char *argv[], uintptr_t _data, void *userdata return 0; } +VERB(verb_set_timezone, "set-timezone", "ZONE", 2, 2, 0, "Set system time zone"); static int verb_set_timezone(int argc, char *argv[], uintptr_t _data, void *userdata) { _cleanup_(sd_bus_error_free) sd_bus_error error = SD_BUS_ERROR_NULL; sd_bus *bus = userdata; @@ -268,6 +272,30 @@ static int verb_set_timezone(int argc, char *argv[], uintptr_t _data, void *user return 0; } +VERB_NOARG(verb_list_timezones, "list-timezones", "Show known time zones"); +static int verb_list_timezones(int argc, char *argv[], uintptr_t _data, void *userdata) { + _cleanup_(sd_bus_error_free) sd_bus_error error = SD_BUS_ERROR_NULL; + sd_bus *bus = userdata; + _cleanup_(sd_bus_message_unrefp) sd_bus_message *reply = NULL; + int r; + _cleanup_strv_free_ char **zones = NULL; + + r = bus_call_method(bus, bus_timedate, "ListTimezones", &error, &reply, NULL); + if (r < 0) + return log_error_errno(r, "Failed to request list of time zones: %s", + bus_error_message(&error, r)); + + r = sd_bus_message_read_strv(reply, &zones); + if (r < 0) + return bus_log_parse_error(r); + + pager_open(arg_pager_flags); + strv_print(zones); + + return 0; +} + +VERB(verb_set_local_rtc, "set-local-rtc", "BOOL", 2, 2, 0, "Control whether RTC is in local time"); static int verb_set_local_rtc(int argc, char *argv[], uintptr_t _data, void *userdata) { _cleanup_(sd_bus_error_free) sd_bus_error error = SD_BUS_ERROR_NULL; sd_bus *bus = userdata; @@ -299,6 +327,7 @@ static int verb_set_local_rtc(int argc, char *argv[], uintptr_t _data, void *use return 0; } +VERB(verb_set_ntp, "set-ntp", "BOOL", 2, 2, 0, "Enable or disable network time synchronization"); static int verb_set_ntp(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; @@ -327,28 +356,6 @@ static int verb_set_ntp(int argc, char *argv[], uintptr_t _data, void *userdata) return 0; } -static int verb_list_timezones(int argc, char *argv[], uintptr_t _data, void *userdata) { - _cleanup_(sd_bus_error_free) sd_bus_error error = SD_BUS_ERROR_NULL; - sd_bus *bus = userdata; - _cleanup_(sd_bus_message_unrefp) sd_bus_message *reply = NULL; - int r; - _cleanup_strv_free_ char **zones = NULL; - - r = bus_call_method(bus, bus_timedate, "ListTimezones", &error, &reply, NULL); - if (r < 0) - return log_error_errno(r, "Failed to request list of time zones: %s", - bus_error_message(&error, r)); - - r = sd_bus_message_read_strv(reply, &zones); - if (r < 0) - return bus_log_parse_error(r); - - pager_open(arg_pager_flags); - strv_print(zones); - - return 0; -} - typedef struct NTPStatusInfo { const char *server_name; char *server_address; @@ -676,7 +683,10 @@ static int on_properties_changed(sd_bus_message *m, void *userdata, sd_bus_error return show_timesync_status_once(sd_bus_message_get_bus(m)); } -static int verb_show_timesync_status(int argc, char *argv[], uintptr_t _data, void *userdata) { +VERB_GROUP("systemd-timesyncd Commands"); + +VERB_NOARG(verb_timesync_status, "timesync-status", "Show status of systemd-timesyncd"); +static int verb_timesync_status(int argc, char *argv[], uintptr_t _data, void *userdata) { _cleanup_(sd_event_unrefp) sd_event *event = NULL; sd_bus *bus = ASSERT_PTR(userdata); int r; @@ -780,6 +790,7 @@ static int print_timesync_property(const char *name, const char *expected_value, return 0; } +VERB_NOARG(verb_show_timesync, "show-timesync", "Show properties of systemd-timesyncd"); static int verb_show_timesync(int argc, char *argv[], uintptr_t _data, void *userdata) { _cleanup_(sd_bus_error_free) sd_bus_error error = SD_BUS_ERROR_NULL; sd_bus *bus = ASSERT_PTR(userdata); @@ -829,6 +840,8 @@ static int parse_ifindex_bus(sd_bus *bus, const char *str) { return i; } +VERB(verb_ntp_servers, "ntp-servers", "INTERFACE SERVER…", 3, VERB_ANY, 0, + "Set the interface specific NTP servers"); static int verb_ntp_servers(int argc, char *argv[], uintptr_t _data, void *userdata) { _cleanup_(sd_bus_error_free) sd_bus_error error = SD_BUS_ERROR_NULL; _cleanup_(sd_bus_message_unrefp) sd_bus_message *req = NULL; @@ -860,6 +873,7 @@ static int verb_ntp_servers(int argc, char *argv[], uintptr_t _data, void *userd return 0; } +VERB(verb_revert, "revert", "INTERFACE", 2, 2, 0, "Revert the interface specific NTP servers"); static int verb_revert(int argc, char *argv[], uintptr_t _data, void *userdata) { _cleanup_(sd_bus_error_free) sd_bus_error error = SD_BUS_ERROR_NULL; sd_bus *bus = ASSERT_PTR(userdata); @@ -880,179 +894,133 @@ static int verb_revert(int argc, char *argv[], uintptr_t _data, void *userdata) static int help(void) { _cleanup_free_ char *link = NULL; + _cleanup_(table_unrefp) Table *verbs = NULL, *verbs2 = NULL, *options = NULL; int r; r = terminal_urlify_man("timedatectl", "1", &link); if (r < 0) return log_oom(); + r = verbs_get_help_table(&verbs); + if (r < 0) + return r; + + r = verbs_get_help_table_group("systemd-timesyncd Commands", &verbs2); + if (r < 0) + return r; + + r = option_parser_get_help_table(&options); + if (r < 0) + return r; + + (void) table_sync_column_widths(0, verbs, verbs2, options); + printf("%s [OPTIONS...] COMMAND ...\n" "\n%sQuery or change system time and date settings.%s\n" - "\nCommands:\n" - " status Show current time settings\n" - " show Show properties of systemd-timedated\n" - " set-time TIME Set system time\n" - " set-timezone ZONE Set system time zone\n" - " list-timezones Show known time zones\n" - " set-local-rtc BOOL Control whether RTC is in local time\n" - " set-ntp BOOL Enable or disable network time synchronization\n" - "\nsystemd-timesyncd Commands:\n" - " timesync-status Show status of systemd-timesyncd\n" - " show-timesync Show properties of systemd-timesyncd\n" - " ntp-servers INTERFACE SERVER…\n" - " Set the interface specific NTP servers\n" - " revert INTERFACE Revert the interface specific NTP servers\n" - "\nOptions:\n" - " -h --help Show this help message\n" - " --version Show package version\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" - " --adjust-system-clock Adjust system clock when changing local RTC mode\n" - " --monitor Monitor status of systemd-timesyncd\n" - " -p --property=NAME Show only properties by this name\n" - " -a --all Show all properties, including empty ones\n" - " --value When showing properties, only print the value\n" - " -P NAME Equivalent to --value --property=NAME\n" - "\nSee the %s for details.\n", + "\nCommands:\n", program_invocation_short_name, ansi_highlight(), - ansi_normal(), - link); + ansi_normal()); + r = table_print_or_warn(verbs); + if (r < 0) + return r; + printf("\nsystemd-timesyncd Commands:\n"); + r = table_print_or_warn(verbs2); + if (r < 0) + return r; + + printf("\nOptions:\n"); + r = table_print_or_warn(options); + if (r < 0) + return r; + + printf("\nSee the %s for details.\n", link); return 0; } -static int verb_help(int argc, char *argv[], uintptr_t _data, void *userdata) { - return help(); -} +VERB_COMMON_HELP_HIDDEN(help); -static int parse_argv(int argc, char *argv[]) { - enum { - ARG_VERSION = 0x100, - ARG_NO_PAGER, - ARG_ADJUST_SYSTEM_CLOCK, - ARG_NO_ASK_PASSWORD, - ARG_MONITOR, - ARG_VALUE, - }; - - static const struct option options[] = { - { "help", no_argument, NULL, 'h' }, - { "version", no_argument, NULL, ARG_VERSION }, - { "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 }, - { "adjust-system-clock", no_argument, NULL, ARG_ADJUST_SYSTEM_CLOCK }, - { "monitor", no_argument, NULL, ARG_MONITOR }, - { "property", required_argument, NULL, 'p' }, - { "value", no_argument, NULL, ARG_VALUE }, - { "all", no_argument, NULL, 'a' }, - {} - }; - - int c, r; +static int parse_argv(int argc, char *argv[], char ***ret_args) { + int r; assert(argc >= 0); assert(argv); - while ((c = getopt_long(argc, argv, "hH:M:p:P:a", options, NULL)) >= 0) - switch (c) { + OptionParser state = { argc, argv }; + const Option *current; + const char *arg; - case 'h': + FOREACH_OPTION_FULL(&state, c, ¤t, &arg, /* on_error= */ return c) + switch (c) { + OPTION_COMMON_HELP: return help(); - case ARG_VERSION: + OPTION_COMMON_VERSION: return version(); - case 'H': - arg_transport = BUS_TRANSPORT_REMOTE; - arg_host = optarg; + OPTION_COMMON_NO_PAGER: + arg_pager_flags |= PAGER_DISABLE; break; - case 'M': - r = parse_machine_argument(optarg, &arg_host, &arg_transport); + OPTION_COMMON_NO_ASK_PASSWORD: + arg_ask_password = false; + break; + + OPTION_COMMON_HOST: + arg_transport = BUS_TRANSPORT_REMOTE; + arg_host = arg; + break; + + OPTION_COMMON_MACHINE: + r = parse_machine_argument(arg, &arg_host, &arg_transport); if (r < 0) return r; break; - case ARG_NO_ASK_PASSWORD: - arg_ask_password = false; - break; - - case ARG_ADJUST_SYSTEM_CLOCK: + OPTION_LONG("adjust-system-clock", NULL, "Adjust system clock when changing local RTC mode"): arg_adjust_system_clock = true; break; - case ARG_NO_PAGER: - arg_pager_flags |= PAGER_DISABLE; - break; - - case ARG_MONITOR: + OPTION_LONG("monitor", NULL, "Monitor status of systemd-timesyncd"): arg_monitor = true; break; - case 'p': - case 'P': - r = strv_extend(&arg_property, optarg); + OPTION('p', "property", "NAME", "Show only properties by this name"): {} + OPTION_SHORT('P', "NAME", "Equivalent to --value --property=NAME"): + r = strv_extend(&arg_property, arg); if (r < 0) return log_oom(); /* If the user asked for a particular property, show it to them, even if empty. */ SET_FLAG(arg_print_flags, BUS_PRINT_PROPERTY_SHOW_EMPTY, true); - if (c == 'p') + if (current->short_code == 'p') break; _fallthrough_; - case ARG_VALUE: + OPTION_LONG("value", NULL, "When showing properties, only print the value"): SET_FLAG(arg_print_flags, BUS_PRINT_PROPERTY_ONLY_VALUE, true); break; - case 'a': + OPTION('a', "all", NULL, "Show all properties, including empty ones"): SET_FLAG(arg_print_flags, BUS_PRINT_PROPERTY_SHOW_EMPTY, true); break; - - case '?': - return -EINVAL; - - default: - assert_not_reached(); } + *ret_args = option_parser_get_args(&state); return 1; } -static int timedatectl_main(sd_bus *bus, int argc, char *argv[]) { - static const Verb verbs[] = { - { "status", VERB_ANY, 1, VERB_DEFAULT, verb_show_status }, - { "show", VERB_ANY, 1, 0, verb_show_properties }, - { "set-time", 2, 2, 0, verb_set_time }, - { "set-timezone", 2, 2, 0, verb_set_timezone }, - { "list-timezones", VERB_ANY, 1, 0, verb_list_timezones }, - { "set-local-rtc", 2, 2, 0, verb_set_local_rtc }, - { "set-ntp", 2, 2, 0, verb_set_ntp }, - { "timesync-status", VERB_ANY, 1, 0, verb_show_timesync_status }, - { "show-timesync", VERB_ANY, 1, 0, verb_show_timesync }, - { "ntp-servers", 3, VERB_ANY, 0, verb_ntp_servers }, - { "revert", 2, 2, 0, verb_revert }, - { "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; + char **args = NULL; int r; setlocale(LC_ALL, ""); log_setup(); - r = parse_argv(argc, argv); + r = parse_argv(argc, argv, &args); if (r <= 0) return r; @@ -1062,7 +1030,7 @@ static int run(int argc, char *argv[]) { (void) sd_bus_set_allow_interactive_authorization(bus, arg_ask_password); - return timedatectl_main(bus, argc, argv); + return dispatch_verb_with_args(args, bus); } DEFINE_MAIN_FUNCTION(run); From ff2fa93ac0f158d03650a34189218981fbec42fc Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Zbigniew=20J=C4=99drzejewski-Szmek?= Date: Thu, 26 Mar 2026 00:04:34 +0100 Subject: [PATCH 11/11] timedatectl: stop using _fallthrough_ gcc and newer clang seem to be fine with it, but clang 14, 16, 18 is unhappy: ../src/timedate/timedatectl.c:1006:25: error: fallthrough annotation does not directly precede switch label _fallthrough_; ^ _fallthrough_ doesn't seem to be used very often in option parsing, so let's remove the use for now. --- src/timedate/timedatectl.c | 6 +++--- 1 file changed, 3 insertions(+), 3 deletions(-) diff --git a/src/timedate/timedatectl.c b/src/timedate/timedatectl.c index 75c142e84e..7d6ca7450a 100644 --- a/src/timedate/timedatectl.c +++ b/src/timedate/timedatectl.c @@ -995,9 +995,9 @@ static int parse_argv(int argc, char *argv[], char ***ret_args) { /* If the user asked for a particular property, show it to them, even if empty. */ SET_FLAG(arg_print_flags, BUS_PRINT_PROPERTY_SHOW_EMPTY, true); - if (current->short_code == 'p') - break; - _fallthrough_; + if (current->short_code == 'P') + SET_FLAG(arg_print_flags, BUS_PRINT_PROPERTY_ONLY_VALUE, true); + break; OPTION_LONG("value", NULL, "When showing properties, only print the value"): SET_FLAG(arg_print_flags, BUS_PRINT_PROPERTY_ONLY_VALUE, true);