From 48fb49f1bdcae3c977883fe267d06c2e4a823046 Mon Sep 17 00:00:00 2001 From: Mike Yuan Date: Mon, 22 Apr 2024 17:33:54 +0800 Subject: [PATCH 1/2] shared/verbs: minor modernization --- src/shared/verbs.c | 18 ++++++++---------- 1 file changed, 8 insertions(+), 10 deletions(-) diff --git a/src/shared/verbs.c b/src/shared/verbs.c index 035337760e..54817252b5 100644 --- a/src/shared/verbs.c +++ b/src/shared/verbs.c @@ -13,22 +13,21 @@ #include "verbs.h" #include "virt.h" -/* Wraps running_in_chroot() which is used in various places, but also adds an environment variable check so external - * processes can reliably force this on. - */ +/* Wraps running_in_chroot() which is used in various places, but also adds an environment variable check + * so external processes can reliably force this on. */ bool running_in_chroot_or_offline(void) { int r; - /* Added to support use cases like rpm-ostree, where from %post scripts we only want to execute "preset", but - * not "start"/"restart" for example. + /* Added to support use cases like rpm-ostree, where from %post scripts we only want to execute "preset", + * but not "start"/"restart" for example. * * See docs/ENVIRONMENT.md for docs. */ r = getenv_bool("SYSTEMD_OFFLINE"); - if (r < 0 && r != -ENXIO) - log_debug_errno(r, "Failed to parse $SYSTEMD_OFFLINE: %m"); - else if (r >= 0) + if (r >= 0) return r > 0; + if (r != -ENXIO) + log_debug_errno(r, "Failed to parse $SYSTEMD_OFFLINE, ignoring: %m"); /* We've had this condition check for a long time which basically checks for legacy chroot case like Fedora's * "mock", which is used for package builds. We don't want to try to start systemd services there, since @@ -40,8 +39,7 @@ bool running_in_chroot_or_offline(void) { */ r = running_in_chroot(); if (r < 0) - log_debug_errno(r, "running_in_chroot(): %m"); - + log_debug_errno(r, "Failed to check if we're running in chroot, assuming not: %m"); return r > 0; } From adaf1f7ea38798d4ddbb3cff6513188fd6e98c9e Mon Sep 17 00:00:00 2001 From: Mike Yuan Date: Mon, 22 Apr 2024 17:40:53 +0800 Subject: [PATCH 2/2] shared/verbs: show list of verbs when missing Replaces #32062 As discussed in #32062, making 'help' the default verb is not very appealing for two reasons: 1) If the verb is missing, showing a help which is pages long isn't really helpful to locate the problem. (https://github.com/systemd/systemd/pull/32062#issuecomment-2064997158) 2) We want to reserve the right to set default verbs to be more useful ones, instead of help. E.g. 'busctl' lists all bus peers by default. So, when there are more than 2 verbs, let's instead add the list of available verbs to the "Command verb required" message, that serves as a hint. That way we try to be friendlier to users, but still make the problem obvious. --- src/shared/verbs.c | 11 +++++++++++ 1 file changed, 11 insertions(+) diff --git a/src/shared/verbs.c b/src/shared/verbs.c index 54817252b5..7e7ac2a255 100644 --- a/src/shared/verbs.c +++ b/src/shared/verbs.c @@ -143,6 +143,17 @@ int dispatch_verb(int argc, char *argv[], const Verb verbs[], void *userdata) { return log_error_errno(SYNTHETIC_ERRNO(EINVAL), "Unknown command verb '%s'.", name); } + _cleanup_free_ char *verb_list = NULL; + size_t i; + + for (i = 0; verbs[i].dispatch; i++) + if (!strextend_with_separator(&verb_list, ", ", verbs[i].verb)) + return log_oom(); + + if (i > 2) + return log_error_errno(SYNTHETIC_ERRNO(EINVAL), + "Command verb required (one of %s).", verb_list); + return log_error_errno(SYNTHETIC_ERRNO(EINVAL), "Command verb required."); }