From e602abba159a2dde3d24aa663319b18143b32665 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Zbigniew=20J=C4=99drzejewski-Szmek?= Date: Sat, 13 Feb 2021 16:08:04 +0100 Subject: [PATCH 1/8] basic/string-table: inline the iterator declaration --- src/basic/string-table.h | 3 +-- 1 file changed, 1 insertion(+), 2 deletions(-) diff --git a/src/basic/string-table.h b/src/basic/string-table.h index aca6a267bd..97c1adc07d 100644 --- a/src/basic/string-table.h +++ b/src/basic/string-table.h @@ -103,9 +103,8 @@ ssize_t string_table_lookup(const char * const *table, size_t len, const char *k #define DUMP_STRING_TABLE(name,type,max) \ do { \ - type _k; \ flockfile(stdout); \ - for (_k = 0; _k < (max); _k++) { \ + for (type _k = 0; _k < (max); _k++) { \ const char *_t; \ _t = name##_to_string(_k); \ if (!_t) \ From 76e5e267fcf4d5c8d1470769b97ad75a7c67394b Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Zbigniew=20J=C4=99drzejewski-Szmek?= Date: Sun, 14 Feb 2021 11:50:08 +0100 Subject: [PATCH 2/8] systemctl: inline iterator declarations --- src/systemctl/systemctl.c | 28 ++++++++-------------------- 1 file changed, 8 insertions(+), 20 deletions(-) diff --git a/src/systemctl/systemctl.c b/src/systemctl/systemctl.c index 16d0555c7c..97a02ff035 100644 --- a/src/systemctl/systemctl.c +++ b/src/systemctl/systemctl.c @@ -492,14 +492,12 @@ static int systemctl_parse_argv(int argc, char *argv[]) { case ARG_VERSION: return version(); - case 't': { - const char *p; - + case 't': if (isempty(optarg)) return log_error_errno(SYNTHETIC_ERRNO(EINVAL), "--type= requires arguments."); - for (p = optarg;;) { + for (const char *p = optarg;;) { _cleanup_free_ char *type = NULL; r = extract_first_word(&p, &type, ",", 0); @@ -533,7 +531,6 @@ static int systemctl_parse_argv(int argc, char *argv[]) { } break; - } case 'P': arg_value = true; @@ -546,10 +543,8 @@ static int systemctl_parse_argv(int argc, char *argv[]) { arg_properties = new0(char*, 1); if (!arg_properties) return log_oom(); - } else { - const char *p; - - for (p = optarg;;) { + } else + for (const char *p = optarg;;) { _cleanup_free_ char *prop = NULL; r = extract_first_word(&p, &prop, ",", 0); @@ -561,7 +556,6 @@ static int systemctl_parse_argv(int argc, char *argv[]) { if (strv_consume(&arg_properties, TAKE_PTR(prop)) < 0) return log_oom(); } - } /* If the user asked for a particular property, show it, even if it is empty. */ arg_all = true; @@ -778,14 +772,12 @@ static int systemctl_parse_argv(int argc, char *argv[]) { arg_boot_loader_entry = empty_to_null(optarg); break; - case ARG_STATE: { - const char *p; - + case ARG_STATE: if (isempty(optarg)) return log_error_errno(SYNTHETIC_ERRNO(EINVAL), "--state= requires arguments."); - for (p = optarg;;) { + for (const char *p = optarg;;) { _cleanup_free_ char *s = NULL; r = extract_first_word(&p, &s, ",", 0); @@ -803,7 +795,6 @@ static int systemctl_parse_argv(int argc, char *argv[]) { return log_oom(); } break; - } case 'r': if (geteuid() != 0) @@ -843,13 +834,11 @@ static int systemctl_parse_argv(int argc, char *argv[]) { arg_with_dependencies = true; break; - case ARG_WHAT: { - const char *p; - + case ARG_WHAT: if (isempty(optarg)) return log_error_errno(SYNTHETIC_ERRNO(EINVAL), "--what= requires arguments."); - for (p = optarg;;) { + for (const char *p = optarg;;) { _cleanup_free_ char *k = NULL; r = extract_first_word(&p, &k, ",", 0); @@ -873,7 +862,6 @@ static int systemctl_parse_argv(int argc, char *argv[]) { } break; - } case ARG_REBOOT_ARG: arg_reboot_argument = optarg; From 86beb21302082e9df0aa8576b89fd29c9e1a43be Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Zbigniew=20J=C4=99drzejewski-Szmek?= Date: Sat, 13 Feb 2021 15:55:10 +0100 Subject: [PATCH 3/8] systemctl,loginctl,machinectl: use a shared helper for arg_signal I seems frivolous to yet another two -util.[ch] files, but the helper should be in shared/ and it doesn't seem to fit anywhere else. --- man/machinectl.xml | 12 +----------- man/standard-options.xml | 15 +++++++++++++++ man/systemctl.xml | 14 +------------- src/login/loginctl.c | 12 ++++-------- src/machine/machinectl.c | 13 ++++--------- src/shared/meson.build | 2 ++ src/shared/parse-argument.c | 25 +++++++++++++++++++++++++ src/shared/parse-argument.h | 4 ++++ src/systemctl/systemctl.c | 14 ++++---------- 9 files changed, 60 insertions(+), 51 deletions(-) create mode 100644 src/shared/parse-argument.c create mode 100644 src/shared/parse-argument.h diff --git a/man/machinectl.xml b/man/machinectl.xml index 9026849559..556fe309dc 100644 --- a/man/machinectl.xml +++ b/man/machinectl.xml @@ -699,17 +699,7 @@ . - - - - - When used with kill, choose - which signal to send to selected processes. Must be one of the - well-known signal specifiers, such as - SIGTERM, SIGINT or - SIGSTOP. If omitted, defaults to - SIGTERM. - + diff --git a/man/standard-options.xml b/man/standard-options.xml index 8ba5b09df8..848439119e 100644 --- a/man/standard-options.xml +++ b/man/standard-options.xml @@ -61,4 +61,19 @@ (for a pretty version of the same, with indentation and line breaks) or off (to turn off JSON output, the default). + + + + + + + When used with kill, choose which signal to send to selected processes. Must + be one of the well-known signal specifiers such as SIGTERM, + SIGINT or SIGSTOP. If omitted, defaults to + . + + The special value help will list the known values and the program will exit + immediately. + + diff --git a/man/systemctl.xml b/man/systemctl.xml index db4e2b2c65..2ed58eb33b 100644 --- a/man/systemctl.xml +++ b/man/systemctl.xml @@ -2067,21 +2067,9 @@ Jan 12 10:46:45 example.com bluetoothd[8900]: gatt-time-server: Input/output err is defined. If omitted, defaults to . - - - - - - - When used with kill, choose which - signal to send to selected processes. Must be one of the - well-known signal specifiers such as SIGTERM, SIGINT or - SIGSTOP. If omitted, defaults to - . - - + diff --git a/src/login/loginctl.c b/src/login/loginctl.c index 918e05e1cd..3e135858a8 100644 --- a/src/login/loginctl.c +++ b/src/login/loginctl.c @@ -22,6 +22,7 @@ #include "main-func.h" #include "memory-util.h" #include "pager.h" +#include "parse-argument.h" #include "parse-util.h" #include "pretty-print.h" #include "process-util.h" @@ -1395,14 +1396,9 @@ static int parse_argv(int argc, char *argv[]) { break; case 's': - if (streq(optarg, "help")) { - DUMP_STRING_TABLE(signal, int, _NSIG); - return 0; - } - - arg_signal = signal_from_string(optarg); - if (arg_signal < 0) - return log_error_errno(arg_signal, "Failed to parse signal string %s.", optarg); + r = parse_signal_argument(optarg, &arg_signal); + if (r <= 0) + return r; break; case 'H': diff --git a/src/machine/machinectl.c b/src/machine/machinectl.c index 3d6048eaff..1ae10e5880 100644 --- a/src/machine/machinectl.c +++ b/src/machine/machinectl.c @@ -39,6 +39,7 @@ #include "mkdir.h" #include "nulstr-util.h" #include "pager.h" +#include "parse-argument.h" #include "parse-util.h" #include "path-util.h" #include "pretty-print.h" @@ -2715,15 +2716,9 @@ static int parse_argv(int argc, char *argv[]) { break; case 's': - if (streq(optarg, "help")) { - DUMP_STRING_TABLE(signal, int, _NSIG); - return 0; - } - - r = signal_from_string(optarg); - if (r < 0) - return log_error_errno(r, "Failed to parse signal string %s.", optarg); - arg_signal = r; + r = parse_signal_argument(optarg, &arg_signal); + if (r <= 0) + return r; break; case ARG_NO_ASK_PASSWORD: diff --git a/src/shared/meson.build b/src/shared/meson.build index 4e47e15c09..6d9a5c5b65 100644 --- a/src/shared/meson.build +++ b/src/shared/meson.build @@ -202,6 +202,8 @@ shared_sources = files(''' output-mode.h pager.c pager.h + parse-argument.c + parse-argument.h pe-header.h pkcs11-util.c pkcs11-util.h diff --git a/src/shared/parse-argument.c b/src/shared/parse-argument.c new file mode 100644 index 0000000000..a30d3adc13 --- /dev/null +++ b/src/shared/parse-argument.c @@ -0,0 +1,25 @@ +/* SPDX-License-Identifier: LGPL-2.1-or-later */ + +#include "parse-argument.h" +#include "signal-util.h" +#include "string-table.h" +#include "string-util.h" + +int parse_signal_argument(const char *s, int *ret) { + int r; + + assert(s); + assert(ret); + + if (streq(s, "help")) { + DUMP_STRING_TABLE(signal, int, _NSIG); + return 0; + } + + r = signal_from_string(s); + if (r < 0) + return log_error_errno(r, "Failed to parse signal string \"%s\".", s); + + *ret = r; + return 1; /* work to do */ +} diff --git a/src/shared/parse-argument.h b/src/shared/parse-argument.h new file mode 100644 index 0000000000..7a5419174d --- /dev/null +++ b/src/shared/parse-argument.h @@ -0,0 +1,4 @@ +/* SPDX-License-Identifier: LGPL-2.1-or-later */ +#pragma once + +int parse_signal_argument(const char *s, int *ret); diff --git a/src/systemctl/systemctl.c b/src/systemctl/systemctl.c index 97a02ff035..0371ff355e 100644 --- a/src/systemctl/systemctl.c +++ b/src/systemctl/systemctl.c @@ -11,6 +11,7 @@ #include "main-func.h" #include "output-mode.h" #include "pager.h" +#include "parse-argument.h" #include "path-util.h" #include "pretty-print.h" #include "rlimit-util.h" @@ -673,16 +674,9 @@ static int systemctl_parse_argv(int argc, char *argv[]) { break; case 's': - if (streq(optarg, "help")) { - DUMP_STRING_TABLE(signal, int, _NSIG); - return 0; - } - - arg_signal = signal_from_string(optarg); - if (arg_signal < 0) - return log_error_errno(SYNTHETIC_ERRNO(EINVAL), - "Failed to parse signal string %s.", - optarg); + r = parse_signal_argument(optarg, &arg_signal); + if (r <= 0) + return r; break; case ARG_NO_ASK_PASSWORD: From 97ab9dfc0d13b142c19335cc53cfd77f96cd2aab Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Zbigniew=20J=C4=99drzejewski-Szmek?= Date: Sat, 13 Feb 2021 16:03:03 +0100 Subject: [PATCH 4/8] systemctl,loginctl,machinectl: add --signal=list This lists numerical signal values: $ systemctl --signal list SIGNAL NAME 1 SIGHUP 2 SIGINT 3 SIGQUIT ... 62 SIGRTMIN+28 63 SIGRTMIN+29 64 SIGRTMIN+30 This is useful when trying to kill e.g. systemd with a specific signal number using kill. kill doesn't accept our fancy signal names like RTMIN+4, so one would have to calculate that value somehow. Doing systemctl --signal list | grep -F RTMIN+4 is a nice way of doing that. --- man/loginctl.xml | 15 ++++++++------- man/standard-options.xml | 3 ++- src/shared/parse-argument.c | 25 +++++++++++++++++++++++++ 3 files changed, 35 insertions(+), 8 deletions(-) diff --git a/man/loginctl.xml b/man/loginctl.xml index d3745ce52d..33683144fa 100644 --- a/man/loginctl.xml +++ b/man/loginctl.xml @@ -334,13 +334,14 @@ - When used with kill-session - or kill-user, choose which signal to send - to selected processes. Must be one of the well known signal - specifiers, such as SIGTERM, - SIGINT or SIGSTOP. - If omitted, defaults to - SIGTERM. + When used with kill-session or kill-user, + choose which signal to send to selected processes. Must be one of the well known signal specifiers, + such as SIGTERM, SIGINT or SIGSTOP. + If omitted, defaults to SIGTERM. + + The special value help will list the known values and the program will exit + immediately, and the special value list will list known values along with the + numerical signal numbers and the program will exit immediately. diff --git a/man/standard-options.xml b/man/standard-options.xml index 848439119e..4565a43b24 100644 --- a/man/standard-options.xml +++ b/man/standard-options.xml @@ -73,7 +73,8 @@ . The special value help will list the known values and the program will exit - immediately. + immediately, and the special value list will list known values along with the + numerical signal numbers and the program will exit immediately. diff --git a/src/shared/parse-argument.c b/src/shared/parse-argument.c index a30d3adc13..ca10d51793 100644 --- a/src/shared/parse-argument.c +++ b/src/shared/parse-argument.c @@ -1,7 +1,9 @@ /* SPDX-License-Identifier: LGPL-2.1-or-later */ +#include "format-table.h" #include "parse-argument.h" #include "signal-util.h" +#include "stdio-util.h" #include "string-table.h" #include "string-util.h" @@ -16,6 +18,29 @@ int parse_signal_argument(const char *s, int *ret) { return 0; } + if (streq(s, "list")) { + _cleanup_(table_unrefp) Table *table = NULL; + + table = table_new("signal", "name"); + if (!table) + return log_oom(); + + for (int i = 1; i < _NSIG; i++) { + r = table_add_many( + table, + TABLE_INT, i, + TABLE_SIGNAL, i); + if (r < 0) + return table_log_add_error(r); + } + + r = table_print(table, NULL); + if (r < 0) + return table_log_print_error(r); + + return 0; + } + r = signal_from_string(s); if (r < 0) return log_error_errno(r, "Failed to parse signal string \"%s\".", s); From 923e2122d09ba577a933b0fb5c7d52cbfa05b080 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Zbigniew=20J=C4=99drzejewski-Szmek?= Date: Sun, 14 Feb 2021 16:02:47 +0100 Subject: [PATCH 5/8] basic/signal-util: drop unneeded extra byte from buffer DECIMAL_STR_MAX() already returns +1 for NULL, so no need to do it a second time. --- src/basic/signal-util.c | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/src/basic/signal-util.c b/src/basic/signal-util.c index c04f8be05b..331295c2c5 100644 --- a/src/basic/signal-util.c +++ b/src/basic/signal-util.c @@ -198,7 +198,7 @@ static const char *const __signal_table[] = { DEFINE_PRIVATE_STRING_TABLE_LOOKUP(__signal, int); const char *signal_to_string(int signo) { - static thread_local char buf[STRLEN("RTMIN+") + DECIMAL_STR_MAX(int) + 1]; + static thread_local char buf[STRLEN("RTMIN+") + DECIMAL_STR_MAX(int)]; const char *name; name = __signal_to_string(signo); From 614b022c76e72a3d20ff48f2bc58ca7c3f86b84f Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Zbigniew=20J=C4=99drzejewski-Szmek?= Date: Sun, 14 Feb 2021 11:39:48 +0100 Subject: [PATCH 6/8] Move and rename parse_path_argument() function This fits better in shared/, and the new parse-argument.c file is a good home for it. --- src/basic/path-util.c | 32 -------------------- src/basic/path-util.h | 2 -- src/core/main.c | 5 +-- src/cryptenroll/cryptenroll.c | 3 +- src/dissect/dissect.c | 3 +- src/firstboot/firstboot.c | 5 +-- src/home/homectl.c | 5 +-- src/journal/journalctl.c | 5 +-- src/machine-id-setup/machine-id-setup-main.c | 3 +- src/nspawn/nspawn.c | 13 ++++---- src/partition/repart.c | 5 +-- src/run/run.c | 3 +- src/shared/parse-argument.c | 31 +++++++++++++++++++ src/shared/parse-argument.h | 1 + src/sysext/sysext.c | 3 +- src/systemctl/systemctl.c | 2 +- src/sysusers/sysusers.c | 5 +-- src/tmpfiles/tmpfiles.c | 5 +-- 18 files changed, 71 insertions(+), 60 deletions(-) diff --git a/src/basic/path-util.c b/src/basic/path-util.c index f7498d0125..f3398418c4 100644 --- a/src/basic/path-util.c +++ b/src/basic/path-util.c @@ -752,38 +752,6 @@ int fsck_exists(const char *fstype) { return executable_is_good(checker); } -int parse_path_argument_and_warn(const char *path, bool suppress_root, char **arg) { - char *p; - int r; - - /* - * This function is intended to be used in command line - * parsers, to handle paths that are passed in. It makes the - * path absolute, and reduces it to NULL if omitted or - * root (the latter optionally). - * - * NOTE THAT THIS WILL FREE THE PREVIOUS ARGUMENT POINTER ON - * SUCCESS! Hence, do not pass in uninitialized pointers. - */ - - if (isempty(path)) { - *arg = mfree(*arg); - return 0; - } - - r = path_make_absolute_cwd(path, &p); - if (r < 0) - return log_error_errno(r, "Failed to parse path \"%s\" and make it absolute: %m", path); - - path_simplify(p, false); - if (suppress_root && empty_or_root(p)) - p = mfree(p); - - free_and_replace(*arg, p); - - return 0; -} - char* dirname_malloc(const char *path) { char *d, *dir, *dir2; diff --git a/src/basic/path-util.h b/src/basic/path-util.h index e7a26c9a84..ba12b03dbe 100644 --- a/src/basic/path-util.h +++ b/src/basic/path-util.h @@ -144,8 +144,6 @@ int fsck_exists(const char *fstype); _ret; \ }) -int parse_path_argument_and_warn(const char *path, bool suppress_root, char **arg); - char* dirname_malloc(const char *path); const char *last_path_component(const char *path); int path_extract_filename(const char *p, char **ret); diff --git a/src/core/main.c b/src/core/main.c index 85bf558e90..8c7bcc2d3d 100644 --- a/src/core/main.c +++ b/src/core/main.c @@ -58,6 +58,7 @@ #include "mount-setup.h" #include "os-util.h" #include "pager.h" +#include "parse-argument.h" #include "parse-util.h" #include "path-util.h" #include "pretty-print.h" @@ -358,7 +359,7 @@ static int parse_proc_cmdline_item(const char *key, const char *value, void *dat return 0; if (path_is_absolute(value)) - (void) parse_path_argument_and_warn(value, false, &arg_early_core_pattern); + (void) parse_path_argument(value, false, &arg_early_core_pattern); else log_warning("Specified core pattern '%s' is not an absolute path, ignoring.", value); @@ -498,7 +499,7 @@ static int parse_proc_cmdline_item(const char *key, const char *value, void *dat if (proc_cmdline_value_missing(key, value)) return 0; - (void) parse_path_argument_and_warn(value, false, &arg_watchdog_device); + (void) parse_path_argument(value, false, &arg_watchdog_device); } else if (proc_cmdline_key_streq(key, "systemd.clock_usec")) { diff --git a/src/cryptenroll/cryptenroll.c b/src/cryptenroll/cryptenroll.c index fa4dfb6ef2..a137a41c9d 100644 --- a/src/cryptenroll/cryptenroll.c +++ b/src/cryptenroll/cryptenroll.c @@ -16,6 +16,7 @@ #include "libfido2-util.h" #include "main-func.h" #include "memory-util.h" +#include "parse-argument.h" #include "parse-util.h" #include "path-util.h" #include "pkcs11-util.h" @@ -323,7 +324,7 @@ static int parse_argv(int argc, char *argv[]) { return log_error_errno(SYNTHETIC_ERRNO(EINVAL), "Too many arguments, refusing."); - r = parse_path_argument_and_warn(argv[optind], false, &arg_node); + r = parse_path_argument(argv[optind], false, &arg_node); if (r < 0) return r; diff --git a/src/dissect/dissect.c b/src/dissect/dissect.c index 0b83d79cc9..e5ca85ffc5 100644 --- a/src/dissect/dissect.c +++ b/src/dissect/dissect.c @@ -22,6 +22,7 @@ #include "mkdir.h" #include "mount-util.h" #include "namespace-util.h" +#include "parse-argument.h" #include "parse-util.h" #include "path-util.h" #include "pretty-print.h" @@ -245,7 +246,7 @@ static int parse_argv(int argc, char *argv[]) { } case ARG_VERITY_DATA: - r = parse_path_argument_and_warn(optarg, false, &arg_verity_settings.data_path); + r = parse_path_argument(optarg, false, &arg_verity_settings.data_path); if (r < 0) return r; break; diff --git a/src/firstboot/firstboot.c b/src/firstboot/firstboot.c index 522d0ff1f0..40f0de28c2 100644 --- a/src/firstboot/firstboot.c +++ b/src/firstboot/firstboot.c @@ -24,6 +24,7 @@ #include "mkdir.h" #include "mount-util.h" #include "os-util.h" +#include "parse-argument.h" #include "parse-util.h" #include "path-util.h" #include "pretty-print.h" @@ -1046,13 +1047,13 @@ static int parse_argv(int argc, char *argv[]) { return version(); case ARG_ROOT: - r = parse_path_argument_and_warn(optarg, true, &arg_root); + r = parse_path_argument(optarg, true, &arg_root); if (r < 0) return r; break; case ARG_IMAGE: - r = parse_path_argument_and_warn(optarg, false, &arg_image); + r = parse_path_argument(optarg, false, &arg_image); if (r < 0) return r; break; diff --git a/src/home/homectl.c b/src/home/homectl.c index bf35fa03f0..c667419a51 100644 --- a/src/home/homectl.c +++ b/src/home/homectl.c @@ -23,6 +23,7 @@ #include "main-func.h" #include "memory-util.h" #include "pager.h" +#include "parse-argument.h" #include "parse-util.h" #include "path-util.h" #include "pkcs11-util.h" @@ -2260,7 +2261,7 @@ static int parse_argv(int argc, char *argv[]) { break; } - r = parse_path_argument_and_warn(optarg, false, &hd); + r = parse_path_argument(optarg, false, &hd); if (r < 0) return r; @@ -2481,7 +2482,7 @@ static int parse_argv(int argc, char *argv[]) { break; } - r = parse_path_argument_and_warn(optarg, false, &v); + r = parse_path_argument(optarg, false, &v); if (r < 0) return r; diff --git a/src/journal/journalctl.c b/src/journal/journalctl.c index 20b7584643..a1b7bee2a3 100644 --- a/src/journal/journalctl.c +++ b/src/journal/journalctl.c @@ -55,6 +55,7 @@ #include "mountpoint-util.h" #include "nulstr-util.h" #include "pager.h" +#include "parse-argument.h" #include "parse-util.h" #include "path-util.h" #include "pcre2-dlopen.h" @@ -719,13 +720,13 @@ static int parse_argv(int argc, char *argv[]) { break; case ARG_ROOT: - r = parse_path_argument_and_warn(optarg, /* suppress_root= */ true, &arg_root); + r = parse_path_argument(optarg, /* suppress_root= */ true, &arg_root); if (r < 0) return r; break; case ARG_IMAGE: - r = parse_path_argument_and_warn(optarg, /* suppress_root= */ false, &arg_image); + r = parse_path_argument(optarg, /* suppress_root= */ false, &arg_image); if (r < 0) return r; break; diff --git a/src/machine-id-setup/machine-id-setup-main.c b/src/machine-id-setup/machine-id-setup-main.c index 6074b1b42e..69fb71cdc2 100644 --- a/src/machine-id-setup/machine-id-setup-main.c +++ b/src/machine-id-setup/machine-id-setup-main.c @@ -10,6 +10,7 @@ #include "log.h" #include "machine-id-setup.h" #include "main-func.h" +#include "parse-argument.h" #include "path-util.h" #include "pretty-print.h" #include "util.h" @@ -76,7 +77,7 @@ static int parse_argv(int argc, char *argv[]) { return version(); case ARG_ROOT: - r = parse_path_argument_and_warn(optarg, true, &arg_root); + r = parse_path_argument(optarg, true, &arg_root); if (r < 0) return r; break; diff --git a/src/nspawn/nspawn.c b/src/nspawn/nspawn.c index e1c55d0542..19734f1c45 100644 --- a/src/nspawn/nspawn.c +++ b/src/nspawn/nspawn.c @@ -78,6 +78,7 @@ #include "nulstr-util.h" #include "os-util.h" #include "pager.h" +#include "parse-argument.h" #include "parse-util.h" #include "path-util.h" #include "pretty-print.h" @@ -791,7 +792,7 @@ static int parse_argv(int argc, char *argv[]) { return version(); case 'D': - r = parse_path_argument_and_warn(optarg, false, &arg_directory); + r = parse_path_argument(optarg, false, &arg_directory); if (r < 0) return r; @@ -799,7 +800,7 @@ static int parse_argv(int argc, char *argv[]) { break; case ARG_TEMPLATE: - r = parse_path_argument_and_warn(optarg, false, &arg_template); + r = parse_path_argument(optarg, false, &arg_template); if (r < 0) return r; @@ -807,7 +808,7 @@ static int parse_argv(int argc, char *argv[]) { break; case 'i': - r = parse_path_argument_and_warn(optarg, false, &arg_image); + r = parse_path_argument(optarg, false, &arg_image); if (r < 0) return r; @@ -815,7 +816,7 @@ static int parse_argv(int argc, char *argv[]) { break; case ARG_OCI_BUNDLE: - r = parse_path_argument_and_warn(optarg, false, &arg_oci_bundle); + r = parse_path_argument(optarg, false, &arg_oci_bundle); if (r < 0) return r; @@ -934,7 +935,7 @@ static int parse_argv(int argc, char *argv[]) { break; case ARG_NETWORK_NAMESPACE_PATH: - r = parse_path_argument_and_warn(optarg, false, &arg_network_namespace_path); + r = parse_path_argument(optarg, false, &arg_network_namespace_path); if (r < 0) return r; @@ -1386,7 +1387,7 @@ static int parse_argv(int argc, char *argv[]) { } case ARG_VERITY_DATA: - r = parse_path_argument_and_warn(optarg, false, &arg_verity_settings.data_path); + r = parse_path_argument(optarg, false, &arg_verity_settings.data_path); if (r < 0) return r; break; diff --git a/src/partition/repart.c b/src/partition/repart.c index 2236c6927f..9828085018 100644 --- a/src/partition/repart.c +++ b/src/partition/repart.c @@ -45,6 +45,7 @@ #include "mkfs-util.h" #include "mount-util.h" #include "parse-util.h" +#include "parse-argument.h" #include "path-util.h" #include "pretty-print.h" #include "proc-cmdline.h" @@ -3623,7 +3624,7 @@ static int parse_argv(int argc, char *argv[]) { break; case ARG_ROOT: - r = parse_path_argument_and_warn(optarg, false, &arg_root); + r = parse_path_argument(optarg, false, &arg_root); if (r < 0) return r; break; @@ -3653,7 +3654,7 @@ static int parse_argv(int argc, char *argv[]) { break; case ARG_DEFINITIONS: - r = parse_path_argument_and_warn(optarg, false, &arg_definitions); + r = parse_path_argument(optarg, false, &arg_definitions); if (r < 0) return r; break; diff --git a/src/run/run.c b/src/run/run.c index 29b182718b..11166d2e78 100644 --- a/src/run/run.c +++ b/src/run/run.c @@ -21,6 +21,7 @@ #include "fd-util.h" #include "format-util.h" #include "main-func.h" +#include "parse-argument.h" #include "parse-util.h" #include "path-util.h" #include "pretty-print.h" @@ -470,7 +471,7 @@ static int parse_argv(int argc, char *argv[]) { break; case ARG_WORKING_DIRECTORY: - r = parse_path_argument_and_warn(optarg, true, &arg_working_directory); + r = parse_path_argument(optarg, true, &arg_working_directory); if (r < 0) return r; diff --git a/src/shared/parse-argument.c b/src/shared/parse-argument.c index ca10d51793..774cb1bc82 100644 --- a/src/shared/parse-argument.c +++ b/src/shared/parse-argument.c @@ -2,11 +2,42 @@ #include "format-table.h" #include "parse-argument.h" +#include "path-util.h" #include "signal-util.h" #include "stdio-util.h" #include "string-table.h" #include "string-util.h" +/* All functions in this file emit warnigs. */ + +int parse_path_argument(const char *path, bool suppress_root, char **arg) { + char *p; + int r; + + /* + * This function is intended to be used in command line parsers, to handle paths that are passed + * in. It makes the path absolute, and reduces it to NULL if omitted or root (the latter optionally). + * + * NOTE THAT THIS WILL FREE THE PREVIOUS ARGUMENT POINTER ON SUCCESS! + * Hence, do not pass in uninitialized pointers. + */ + + if (isempty(path)) { + *arg = mfree(*arg); + return 0; + } + + r = path_make_absolute_cwd(path, &p); + if (r < 0) + return log_error_errno(r, "Failed to parse path \"%s\" and make it absolute: %m", path); + + path_simplify(p, false); + if (suppress_root && empty_or_root(p)) + p = mfree(p); + + return free_and_replace(*arg, p); +} + int parse_signal_argument(const char *s, int *ret) { int r; diff --git a/src/shared/parse-argument.h b/src/shared/parse-argument.h index 7a5419174d..4ec334fe5e 100644 --- a/src/shared/parse-argument.h +++ b/src/shared/parse-argument.h @@ -1,4 +1,5 @@ /* SPDX-License-Identifier: LGPL-2.1-or-later */ #pragma once +int parse_path_argument(const char *path, bool suppress_root, char **arg); int parse_signal_argument(const char *s, int *ret); diff --git a/src/sysext/sysext.c b/src/sysext/sysext.c index 1ce9939afb..ff8d58902a 100644 --- a/src/sysext/sysext.c +++ b/src/sysext/sysext.c @@ -22,6 +22,7 @@ #include "mountpoint-util.h" #include "os-util.h" #include "pager.h" +#include "parse-argument.h" #include "parse-util.h" #include "pretty-print.h" #include "process-util.h" @@ -943,7 +944,7 @@ static int parse_argv(int argc, char *argv[]) { break; case ARG_ROOT: - r = parse_path_argument_and_warn(optarg, false, &arg_root); + r = parse_path_argument(optarg, false, &arg_root); if (r < 0) return r; break; diff --git a/src/systemctl/systemctl.c b/src/systemctl/systemctl.c index 0371ff355e..3ecd20cc74 100644 --- a/src/systemctl/systemctl.c +++ b/src/systemctl/systemctl.c @@ -638,7 +638,7 @@ static int systemctl_parse_argv(int argc, char *argv[]) { break; case ARG_ROOT: - r = parse_path_argument_and_warn(optarg, false, &arg_root); + r = parse_path_argument(optarg, false, &arg_root); if (r < 0) return r; break; diff --git a/src/sysusers/sysusers.c b/src/sysusers/sysusers.c index 969896c0b5..4e231be856 100644 --- a/src/sysusers/sysusers.c +++ b/src/sysusers/sysusers.c @@ -17,6 +17,7 @@ #include "mount-util.h" #include "nscd-flush.h" #include "pager.h" +#include "parse-argument.h" #include "path-util.h" #include "pretty-print.h" #include "selinux-util.h" @@ -1813,7 +1814,7 @@ static int parse_argv(int argc, char *argv[]) { break; case ARG_ROOT: - r = parse_path_argument_and_warn(optarg, /* suppress_root= */ false, &arg_root); + r = parse_path_argument(optarg, /* suppress_root= */ false, &arg_root); if (r < 0) return r; break; @@ -1823,7 +1824,7 @@ static int parse_argv(int argc, char *argv[]) { return log_error_errno(SYNTHETIC_ERRNO(EOPNOTSUPP), "This systemd-sysusers version is compiled without support for --image=."); #else - r = parse_path_argument_and_warn(optarg, /* suppress_root= */ false, &arg_image); + r = parse_path_argument(optarg, /* suppress_root= */ false, &arg_image); if (r < 0) return r; break; diff --git a/src/tmpfiles/tmpfiles.c b/src/tmpfiles/tmpfiles.c index 97de00e3dc..846e456551 100644 --- a/src/tmpfiles/tmpfiles.c +++ b/src/tmpfiles/tmpfiles.c @@ -46,6 +46,7 @@ #include "mountpoint-util.h" #include "offline-passwd.h" #include "pager.h" +#include "parse-argument.h" #include "parse-util.h" #include "path-lookup.h" #include "path-util.h" @@ -3114,7 +3115,7 @@ static int parse_argv(int argc, char *argv[]) { break; case ARG_ROOT: - r = parse_path_argument_and_warn(optarg, /* suppress_root= */ false, &arg_root); + r = parse_path_argument(optarg, /* suppress_root= */ false, &arg_root); if (r < 0) return r; break; @@ -3124,7 +3125,7 @@ static int parse_argv(int argc, char *argv[]) { return log_error_errno(SYNTHETIC_ERRNO(EOPNOTSUPP), "This systemd-tmpfiles version is compiled without support for --image=."); #else - r = parse_path_argument_and_warn(optarg, /* suppress_root= */ false, &arg_image); + r = parse_path_argument(optarg, /* suppress_root= */ false, &arg_image); if (r < 0) return r; #endif From b1e8f46c318d88df8e0939e6cf2428ba4c343957 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Zbigniew=20J=C4=99drzejewski-Szmek?= Date: Sun, 14 Feb 2021 11:45:01 +0100 Subject: [PATCH 7/8] Move and rename parse_json_argument() function json.[ch] is a very generic implementation, and cmdline argument parsing doesn't fit there. --- src/busctl/busctl.c | 3 ++- src/coredump/coredumpctl.c | 3 ++- src/dissect/dissect.c | 2 +- src/home/homectl.c | 2 +- src/partition/repart.c | 2 +- src/shared/json.c | 21 --------------------- src/shared/json.h | 2 -- src/shared/parse-argument.c | 21 +++++++++++++++++++++ src/shared/parse-argument.h | 3 +++ src/sysext/sysext.c | 2 +- 10 files changed, 32 insertions(+), 29 deletions(-) diff --git a/src/busctl/busctl.c b/src/busctl/busctl.c index 7774aeac7c..6a492ebd02 100644 --- a/src/busctl/busctl.c +++ b/src/busctl/busctl.c @@ -21,6 +21,7 @@ #include "log.h" #include "main-func.h" #include "pager.h" +#include "parse-argument.h" #include "parse-util.h" #include "path-util.h" #include "pretty-print.h" @@ -2523,7 +2524,7 @@ static int parse_argv(int argc, char *argv[]) { break; case ARG_JSON: - r = json_parse_cmdline_parameter_and_warn(optarg, &arg_json_format_flags); + r = parse_json_argument(optarg, &arg_json_format_flags); if (r <= 0) return r; diff --git a/src/coredump/coredumpctl.c b/src/coredump/coredumpctl.c index 46183451de..20cd90bd56 100644 --- a/src/coredump/coredumpctl.c +++ b/src/coredump/coredumpctl.c @@ -25,6 +25,7 @@ #include "macro.h" #include "main-func.h" #include "pager.h" +#include "parse-argument.h" #include "parse-util.h" #include "path-util.h" #include "pretty-print.h" @@ -320,7 +321,7 @@ static int parse_argv(int argc, char *argv[]) { break; case ARG_JSON: - r = json_parse_cmdline_parameter_and_warn(optarg, &arg_json_format_flags); + r = parse_json_argument(optarg, &arg_json_format_flags); if (r <= 0) return r; diff --git a/src/dissect/dissect.c b/src/dissect/dissect.c index e5ca85ffc5..f8f34dcc4d 100644 --- a/src/dissect/dissect.c +++ b/src/dissect/dissect.c @@ -260,7 +260,7 @@ static int parse_argv(int argc, char *argv[]) { break; case ARG_JSON: - r = json_parse_cmdline_parameter_and_warn(optarg, &arg_json_format_flags); + r = parse_json_argument(optarg, &arg_json_format_flags); if (r <= 0) return r; diff --git a/src/home/homectl.c b/src/home/homectl.c index c667419a51..83fc86efe3 100644 --- a/src/home/homectl.c +++ b/src/home/homectl.c @@ -3234,7 +3234,7 @@ static int parse_argv(int argc, char *argv[]) { break; case ARG_JSON: - r = json_parse_cmdline_parameter_and_warn(optarg, &arg_json_format_flags); + r = parse_json_argument(optarg, &arg_json_format_flags); if (r <= 0) return r; diff --git a/src/partition/repart.c b/src/partition/repart.c index 9828085018..78c31187ea 100644 --- a/src/partition/repart.c +++ b/src/partition/repart.c @@ -3688,7 +3688,7 @@ static int parse_argv(int argc, char *argv[]) { } case ARG_JSON: - r = json_parse_cmdline_parameter_and_warn(optarg, &arg_json_format_flags); + r = parse_json_argument(optarg, &arg_json_format_flags); if (r <= 0) return r; diff --git a/src/shared/json.c b/src/shared/json.c index b8f8292e74..cc79375410 100644 --- a/src/shared/json.c +++ b/src/shared/json.c @@ -4459,27 +4459,6 @@ int json_variant_unhex(JsonVariant *v, void **ret, size_t *ret_size) { return unhexmem(json_variant_string(v), (size_t) -1, ret, ret_size); } -int json_parse_cmdline_parameter_and_warn(const char *s, JsonFormatFlags *ret) { - assert(s); - assert(ret); - - if (streq(s, "pretty")) - *ret = JSON_FORMAT_PRETTY|JSON_FORMAT_COLOR_AUTO; - else if (streq(s, "short")) - *ret = JSON_FORMAT_NEWLINE; - else if (streq(s, "off")) - *ret = JSON_FORMAT_OFF; - else if (streq(s, "help")) { - puts("pretty\n" - "short\n" - "off"); - return 0; /* 0 means → we showed a brief help, exit now */ - } else - return log_error_errno(SYNTHETIC_ERRNO(EINVAL), "Unknown argument to --json= switch: %s", s); - - return 1; /* 1 means → properly parsed */ -} - static const char* const json_variant_type_table[_JSON_VARIANT_TYPE_MAX] = { [JSON_VARIANT_STRING] = "string", [JSON_VARIANT_INTEGER] = "integer", diff --git a/src/shared/json.h b/src/shared/json.h index a335dd3623..d429066761 100644 --- a/src/shared/json.h +++ b/src/shared/json.h @@ -357,7 +357,5 @@ int json_log_internal(JsonVariant *variant, int level, int error, const char *fi int json_variant_unbase64(JsonVariant *v, void **ret, size_t *ret_size); int json_variant_unhex(JsonVariant *v, void **ret, size_t *ret_size); -int json_parse_cmdline_parameter_and_warn(const char *s, JsonFormatFlags *ret); - const char *json_variant_type_to_string(JsonVariantType t); JsonVariantType json_variant_type_from_string(const char *s); diff --git a/src/shared/parse-argument.c b/src/shared/parse-argument.c index 774cb1bc82..cd1d0dde21 100644 --- a/src/shared/parse-argument.c +++ b/src/shared/parse-argument.c @@ -10,6 +10,27 @@ /* All functions in this file emit warnigs. */ +int parse_json_argument(const char *s, JsonFormatFlags *ret) { + assert(s); + assert(ret); + + if (streq(s, "pretty")) + *ret = JSON_FORMAT_PRETTY|JSON_FORMAT_COLOR_AUTO; + else if (streq(s, "short")) + *ret = JSON_FORMAT_NEWLINE; + else if (streq(s, "off")) + *ret = JSON_FORMAT_OFF; + else if (streq(s, "help")) { + puts("pretty\n" + "short\n" + "off"); + return 0; /* 0 means → we showed a brief help, exit now */ + } else + return log_error_errno(SYNTHETIC_ERRNO(EINVAL), "Unknown argument to --json= switch: %s", s); + + return 1; /* 1 means → properly parsed */ +} + int parse_path_argument(const char *path, bool suppress_root, char **arg) { char *p; int r; diff --git a/src/shared/parse-argument.h b/src/shared/parse-argument.h index 4ec334fe5e..28b58cc2e2 100644 --- a/src/shared/parse-argument.h +++ b/src/shared/parse-argument.h @@ -1,5 +1,8 @@ /* SPDX-License-Identifier: LGPL-2.1-or-later */ #pragma once +#include "json.h" + +int parse_json_argument(const char *s, JsonFormatFlags *ret); int parse_path_argument(const char *path, bool suppress_root, char **arg); int parse_signal_argument(const char *s, int *ret); diff --git a/src/sysext/sysext.c b/src/sysext/sysext.c index ff8d58902a..57a36d478a 100644 --- a/src/sysext/sysext.c +++ b/src/sysext/sysext.c @@ -950,7 +950,7 @@ static int parse_argv(int argc, char *argv[]) { break; case ARG_JSON: - r = json_parse_cmdline_parameter_and_warn(optarg, &arg_json_format_flags); + r = parse_json_argument(optarg, &arg_json_format_flags); if (r <= 0) return r; From 1751efdf6fd2d2f1b34014a1f19a4db3b5083d27 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Zbigniew=20J=C4=99drzejewski-Szmek?= Date: Sun, 14 Feb 2021 13:35:32 +0100 Subject: [PATCH 8/8] test-parse-argument: add a test for the three parse_*_argument() functions This mostly tests the return values and that the xsprintf buffers are big enough. --- src/test/meson.build | 2 ++ src/test/test-parse-argument.c | 63 ++++++++++++++++++++++++++++++++++ 2 files changed, 65 insertions(+) create mode 100644 src/test/test-parse-argument.c diff --git a/src/test/meson.build b/src/test/meson.build index 93870e8773..5bf1ca3490 100644 --- a/src/test/meson.build +++ b/src/test/meson.build @@ -222,6 +222,8 @@ tests += [ [['src/test/test-extract-word.c']], + [['src/test/test-parse-argument.c']], + [['src/test/test-parse-util.c'], [], [libseccomp]], diff --git a/src/test/test-parse-argument.c b/src/test/test-parse-argument.c new file mode 100644 index 0000000000..4081a9f25a --- /dev/null +++ b/src/test/test-parse-argument.c @@ -0,0 +1,63 @@ +/* SPDX-License-Identifier: LGPL-2.1-or-later */ + +#include "parse-argument.h" +#include "stdio-util.h" +#include "tests.h" + +static void test_parse_json_argument(void) { + log_info("/* %s */", __func__); + + JsonFormatFlags flags = JSON_FORMAT_PRETTY; + + assert_se(parse_json_argument("help", &flags) == 0); + assert_se(flags == JSON_FORMAT_PRETTY); + + assert_se(parse_json_argument("off", &flags) == 1); + assert_se(flags == JSON_FORMAT_OFF); +} + +static void test_parse_path_argument(void) { + log_info("/* %s */", __func__); + + _cleanup_free_ char *path = NULL; + + assert_se(parse_path_argument("help", false, &path) == 0); + assert_se(streq(basename(path), "help")); + + assert_se(parse_path_argument("/", false, &path) == 0); + assert_se(streq(path, "/")); + + assert_se(parse_path_argument("/", true, &path) == 0); + assert_se(path == NULL); +} + +static void test_parse_signal_argument(void) { + log_info("/* %s */", __func__); + + int signal = -1; + + assert_se(parse_signal_argument("help", &signal) == 0); + assert_se(signal == -1); + + assert_se(parse_signal_argument("list", &signal) == 0); + assert_se(signal == -1); + + assert_se(parse_signal_argument("SIGABRT", &signal) == 1); + assert_se(signal == SIGABRT); + + assert_se(parse_signal_argument("ABRT", &signal) == 1); + assert_se(signal == SIGABRT); + + char buf[DECIMAL_STR_MAX(int)]; + xsprintf(buf, "%d", SIGABRT); + assert_se(parse_signal_argument(buf, &signal) == 1); + assert_se(signal == SIGABRT); +} + +int main(int argc, char *argv[]) { + test_setup_logging(LOG_INFO); + + test_parse_json_argument(); + test_parse_path_argument(); + test_parse_signal_argument(); +}