]> git.ipfire.org Git - thirdparty/systemd.git/commitdiff
systemctl: drop translation of method names to descriptions in error message
authorZbigniew Jędrzejewski-Szmek <zbyszek@in.waw.pl>
Tue, 31 May 2022 07:49:29 +0000 (09:49 +0200)
committerZbigniew Jędrzejewski-Szmek <zbyszek@in.waw.pl>
Wed, 1 Jun 2022 07:23:55 +0000 (09:23 +0200)
We had yet-another table of descriptive strings to use in error messages.
I started thinking how to synchronize them with the strings in logind, but
ultimately I think it's better to remove those altogether. Those strings
should almost never be used: normally if the call fails, logind will provide
an error message itself, which is probably more detailed than what we can
figure out on the client side. And the most important part that we want to
show here is what exactly we called, in particular RebootWithFlags vs. Reboot,
etc. By using the "descriptive strings" we were obfuscating this. So let's just
simplify our code and print the actual method name, since this is more useful
as an error statement that is googlable and unique.

While at it, let's print the correct method name ;)

src/systemctl/systemctl-logind.c

index 239bc777ad78f69f43e1fe9464565c1041cfbc0c..1c3b68f09f17a31cb038ecb8d0e0df225b48c9ff 100644 (file)
@@ -39,27 +39,23 @@ static int logind_set_wall_message(sd_bus *bus) {
 /* Ask systemd-logind, which might grant access to unprivileged users through polkit */
 int logind_reboot(enum action a) {
 #if ENABLE_LOGIND
-        static const struct {
-                const char *method;
-                const char *description;
-        } actions[_ACTION_MAX] = {
-                [ACTION_POWEROFF]               = { "PowerOff",             "power off system"                },
-                [ACTION_REBOOT]                 = { "Reboot",               "reboot system"                   },
-                [ACTION_KEXEC]                  = { "Reboot",               "kexec reboot system"             },
-                [ACTION_HALT]                   = { "Halt",                 "halt system"                     },
-                [ACTION_SUSPEND]                = { "Suspend",              "suspend system"                  },
-                [ACTION_HIBERNATE]              = { "Hibernate",            "hibernate system"                },
-                [ACTION_HYBRID_SLEEP]           = { "HybridSleep",          "put system into hybrid sleep"    },
-                [ACTION_SUSPEND_THEN_HIBERNATE] = { "SuspendThenHibernate", "suspend system, hibernate later" },
+        static const char* actions[_ACTION_MAX] = {
+                [ACTION_POWEROFF]               = "PowerOff",
+                [ACTION_REBOOT]                 = "Reboot",
+                [ACTION_KEXEC]                  = "Reboot",
+                [ACTION_HALT]                   = "Halt",
+                [ACTION_SUSPEND]                = "Suspend",
+                [ACTION_HIBERNATE]              = "Hibernate",
+                [ACTION_HYBRID_SLEEP]           = "HybridSleep",
+                [ACTION_SUSPEND_THEN_HIBERNATE] = "SuspendThenHibernate",
         };
 
         _cleanup_(sd_bus_error_free) sd_bus_error error = SD_BUS_ERROR_NULL;
-        const char *method_with_flags;
         uint64_t flags = 0;
         sd_bus *bus;
         int r;
 
-        if (a < 0 || a >= _ACTION_MAX || !actions[a].method)
+        if (a < 0 || a >= _ACTION_MAX || !actions[a])
                 return -EINVAL;
 
         r = acquire_bus(BUS_FULL, &bus);
@@ -69,7 +65,10 @@ int logind_reboot(enum action a) {
         polkit_agent_open_maybe();
         (void) logind_set_wall_message(bus);
 
-        log_debug("%s org.freedesktop.login1.Manager %s dbus call.", arg_dry_run ? "Would execute" : "Executing", actions[a].method);
+        const char *method_with_flags = strjoina(actions[a], "WithFlags");
+
+        log_debug("%s org.freedesktop.login1.Manager %s dbus call.",
+                  arg_dry_run ? "Would execute" : "Executing", method_with_flags);
 
         if (arg_dry_run)
                 return 0;
@@ -77,21 +76,19 @@ int logind_reboot(enum action a) {
         SET_FLAG(flags, SD_LOGIND_ROOT_CHECK_INHIBITORS, arg_check_inhibitors > 0);
         SET_FLAG(flags, SD_LOGIND_REBOOT_VIA_KEXEC, a == ACTION_KEXEC);
 
-        method_with_flags = strjoina(actions[a].method, "WithFlags");
-
         r = bus_call_method(bus, bus_login_mgr, method_with_flags, &error, NULL, "t", flags);
         if (r >= 0)
                 return 0;
         if (!sd_bus_error_has_name(&error, SD_BUS_ERROR_UNKNOWN_METHOD))
-                return log_error_errno(r, "Failed to %s via logind: %s", actions[a].description, bus_error_message(&error, r));
+                return log_error_errno(r, "Call to %s failed: %s", actions[a], bus_error_message(&error, r));
 
-        /* Fallback to original methods in case there is older version of systemd-logind */
-        log_debug("Method %s not available: %s. Falling back to %s", method_with_flags, bus_error_message(&error, r), actions[a].method);
+        /* Fall back to original methods in case there is an older version of systemd-logind */
+        log_debug("Method %s not available: %s. Falling back to %s", method_with_flags, bus_error_message(&error, r), actions[a]);
         sd_bus_error_free(&error);
 
-        r = bus_call_method(bus, bus_login_mgr, actions[a].method, &error, NULL, "b", arg_ask_password);
+        r = bus_call_method(bus, bus_login_mgr, actions[a], &error, NULL, "b", arg_ask_password);
         if (r < 0)
-                return log_error_errno(r, "Failed to %s via logind: %s", actions[a].description, bus_error_message(&error, r));
+                return log_error_errno(r, "Call to %s failed: %s", actions[a], bus_error_message(&error, r));
 
         return 0;
 #else