busctl: verify args early and always print results to stdout

We would print the error sometimes to stdout and sometimes to stderr. It *is*
useful to get the message if one of the names is not found on the bus to
stdout, so that this shows out in the pager. So let's do verification of args
early to catch invalid arguments, and then if we receive an error over the bus
(most likely that the name is not activatable), let's print to stdout so it
gets paged. E.g. 'busctl tree org.freedesktop.systemd1 org.freedesktop.systemd2'
gives a nicely usable output.

diff --git a/TODO b/TODO
index e4182588fe87320d30efdb7c670e9188b33a43a6..e962db26ca6a2ac988305210483f4500c6fa22ed 100644 (file)
--- a/TODO
+++ b/TODO
@@ -4,9 +4,6 @@ Bugfixes:
   manager or system manager can be always set. It would be better to reject
   them when parsing config.
 
-* busctl prints errors to stdout:
-  busctl tree org.freedesktop.systemd1  /org/freedesktop/systemd1
-
 External:
 
 * Fedora: add an rpmlint check that verifies that all unit files in the RPM are listed in %systemd_post macros.

diff --git a/src/busctl/busctl.c b/src/busctl/busctl.c
index a4132b3345fe9045a2fb2869c32b10c8a44809a6..4879b466e30440defeb7c22cdb10eb5984448d9a 100644 (file)
--- a/src/busctl/busctl.c
+++ b/src/busctl/busctl.c
@@ -462,7 +462,7 @@ static int on_path(const char *path, void *userdata) {
         return 0;
 }
 
-static int find_nodes(sd_bus *bus, const char *service, const char *path, Set *paths, bool many) {
+static int find_nodes(sd_bus *bus, const char *service, const char *path, Set *paths) {
         static const XMLIntrospectOps ops = {
                 .on_path = on_path,
         };
@@ -476,12 +476,10 @@ static int find_nodes(sd_bus *bus, const char *service, const char *path, Set *p
                                "org.freedesktop.DBus.Introspectable", "Introspect",
                                &error, &reply, "");
         if (r < 0) {
-                if (many)
-                        printf("Failed to introspect object %s of service %s: %s\n",
-                               path, service, bus_error_message(&error, r));
-                else
-                        log_error_errno(r, "Failed to introspect object %s of service %s: %s",
-                                        path, service, bus_error_message(&error, r));
+                printf("%sFailed to introspect object %s of service %s: %s%s\n",
+                       ansi_highlight_red(),
+                       path, service, bus_error_message(&error, r),
+                       ansi_normal());
                 return r;
         }
 
@@ -492,7 +490,7 @@ static int find_nodes(sd_bus *bus, const char *service, const char *path, Set *p
         return parse_xml_introspect(path, xml, &ops, paths);
 }
 
-static int tree_one(sd_bus *bus, const char *service, const char *prefix, bool many) {
+static int tree_one(sd_bus *bus, const char *service, const char *prefix) {
         _cleanup_set_free_ Set *paths = NULL, *done = NULL, *failed = NULL;
         _cleanup_free_ char **l = NULL;
         int r;
@@ -521,7 +519,7 @@ static int tree_one(sd_bus *bus, const char *service, const char *prefix, bool m
                     set_contains(failed, p))
                         continue;
 
-                q = find_nodes(bus, service, p, paths, many);
+                q = find_nodes(bus, service, p, paths);
                 if (q < 0 && r >= 0)
                         r = q;
 
@@ -550,6 +548,12 @@ static int tree(int argc, char **argv, void *userdata) {
         char **i;
         int r = 0;
 
+        /* Do superficial verification of arguments before even opening the bus */
+        STRV_FOREACH(i, strv_skip(argv, 1))
+                if (!sd_bus_service_name_is_valid(*i))
+                        return log_error_errno(SYNTHETIC_ERRNO(EINVAL),
+                                               "Invalid bus service name: %s", *i);
+
         if (!arg_unique && !arg_acquired)
                 arg_acquired = true;
 
@@ -581,14 +585,14 @@ static int tree(int argc, char **argv, void *userdata) {
 
                         printf("Service %s%s%s:\n", ansi_highlight(), *i, ansi_normal());
 
-                        q = tree_one(bus, *i, NULL, true);
+                        q = tree_one(bus, *i, NULL);
                         if (q < 0 && r >= 0)
                                 r = q;
 
                         not_first = true;
                 }
-        } else {
-                STRV_FOREACH(i, argv+1) {
+        } else
+                STRV_FOREACH(i, strv_skip(argv, 1)) {
                         int q;
 
                         if (i > argv+1)
@@ -599,11 +603,10 @@ static int tree(int argc, char **argv, void *userdata) {
                                 printf("Service %s%s%s:\n", ansi_highlight(), *i, ansi_normal());
                         }
 
-                        q = tree_one(bus, *i, NULL, !!argv[2]);
+                        q = tree_one(bus, *i, NULL);
                         if (q < 0 && r >= 0)
                                 r = q;
                 }
-        }
 
         return r;
 }