From aacbc2acb94d95ccedb69cc1bcabc31833d48aa2 Mon Sep 17 00:00:00 2001 From: Lennart Poettering Date: Mon, 27 May 2024 11:36:48 +0200 Subject: [PATCH 1/5] json: move empty string check from json_parse_file_at() to json_parse_with_source() That way this error is generalized and applies to either function, since one is just a wrapper to the other, normalizing behaviour. --- src/libsystemd/sd-json/sd-json.c | 6 +++--- 1 file changed, 3 insertions(+), 3 deletions(-) diff --git a/src/libsystemd/sd-json/sd-json.c b/src/libsystemd/sd-json/sd-json.c index 3a0b838c51..2c5c0a35c9 100644 --- a/src/libsystemd/sd-json/sd-json.c +++ b/src/libsystemd/sd-json/sd-json.c @@ -3378,6 +3378,9 @@ _public_ int sd_json_parse_with_source( _cleanup_(json_source_unrefp) JsonSource *s = NULL; + if (isempty(input)) + return -ENODATA; + if (source) { s = json_source_new(source); if (!s) @@ -3447,9 +3450,6 @@ _public_ int sd_json_parse_file_at( if (r < 0) return r; - if (isempty(text)) - return -ENODATA; - return sd_json_parse_with_source(text, path, flags, ret, reterr_line, reterr_column); } From cd4e9166bb5f0b970367c54e198817f94213a376 Mon Sep 17 00:00:00 2001 From: Lennart Poettering Date: Wed, 8 May 2024 10:40:10 +0200 Subject: [PATCH 2/5] varlinkctl: add "-q" switch for suppressing varlinkctl output --- man/varlinkctl.xml | 11 +++++++++++ src/varlinkctl/varlinkctl.c | 22 ++++++++++++++++++++-- 2 files changed, 31 insertions(+), 2 deletions(-) diff --git a/man/varlinkctl.xml b/man/varlinkctl.xml index 0dedc1e4db..f21e513cb0 100644 --- a/man/varlinkctl.xml +++ b/man/varlinkctl.xml @@ -239,6 +239,17 @@ + + + + + + Suppress output of method call replies. + + + + + diff --git a/src/varlinkctl/varlinkctl.c b/src/varlinkctl/varlinkctl.c index 739770d296..676fb93054 100644 --- a/src/varlinkctl/varlinkctl.c +++ b/src/varlinkctl/varlinkctl.c @@ -21,6 +21,7 @@ static sd_json_format_flags_t arg_json_format_flags = SD_JSON_FORMAT_OFF; static PagerFlags arg_pager_flags = 0; static VarlinkMethodFlags arg_method_flags = 0; static bool arg_collect = false; +static bool arg_quiet = false; static int help(void) { _cleanup_free_ char *link = NULL; @@ -56,6 +57,7 @@ static int help(void) { " --oneway Do not request response\n" " --json=MODE Output as JSON\n" " -j Same as --json=pretty on tty, --json=short otherwise\n" + " -q --quiet Do not output method reply\n" "\nSee the %2$s for details.\n", program_invocation_short_name, link, @@ -90,6 +92,7 @@ static int parse_argv(int argc, char *argv[]) { { "oneway", no_argument, NULL, ARG_ONEWAY }, { "json", required_argument, NULL, ARG_JSON }, { "collect", no_argument, NULL, ARG_COLLECT }, + { "quiet", no_argument, NULL, 'q' }, {}, }; @@ -98,7 +101,7 @@ static int parse_argv(int argc, char *argv[]) { assert(argc >= 0); assert(argv); - while ((c = getopt_long(argc, argv, "hj", options, NULL)) >= 0) + while ((c = getopt_long(argc, argv, "hjq", options, NULL)) >= 0) switch (c) { @@ -135,6 +138,10 @@ static int parse_argv(int argc, char *argv[]) { arg_json_format_flags = SD_JSON_FORMAT_PRETTY_AUTO|SD_JSON_FORMAT_COLOR_AUTO; break; + case 'q': + arg_quiet = true; + break; + case '?': return -EINVAL; @@ -435,7 +442,9 @@ static int reply_callback( } else r = 0; - sd_json_variant_dump(parameters, arg_json_format_flags, stdout, NULL); + if (!arg_quiet) + sd_json_variant_dump(parameters, arg_json_format_flags, stdout, NULL); + return r; } @@ -490,6 +499,9 @@ static int verb_call(int argc, char *argv[], void *userdata) { } else r = 0; + if (arg_quiet) + return r; + pager_open(arg_pager_flags); sd_json_variant_dump(reply, arg_json_format_flags, stdout, NULL); return r; @@ -552,6 +564,9 @@ static int verb_call(int argc, char *argv[], void *userdata) { } else r = 0; + if (arg_quiet) + return r; + pager_open(arg_pager_flags); sd_json_variant_dump(reply, arg_json_format_flags, stdout, NULL); @@ -598,6 +613,9 @@ static int verb_validate_idl(int argc, char *argv[], void *userdata) { if (r < 0) return log_error_errno(r, "Failed to check interface for consistency: %m"); + if (arg_quiet) + return 0; + pager_open(arg_pager_flags); r = varlink_idl_dump(stdout, /* use_colors= */ -1, vi); From 273b96f9fa61adedbea22351293c78594c1af0fb Mon Sep 17 00:00:00 2001 From: Lennart Poettering Date: Mon, 27 May 2024 11:28:48 +0200 Subject: [PATCH 3/5] varlinkctl: tell user we are expecting method call parameters on STDIN When calling a method and the user hasn't provided any method call parameters on the command line we expect them on stdin instead. This might be confusing for people using varlinkctl for the first time, since omitting the parameters will just throw you at a blinking cursor. Let's be a bit more helpful, and show a friendly message when we are connected to a TTY (i.e. run interactively). --- src/varlinkctl/varlinkctl.c | 3 +++ 1 file changed, 3 insertions(+) diff --git a/src/varlinkctl/varlinkctl.c b/src/varlinkctl/varlinkctl.c index 676fb93054..a994c640ca 100644 --- a/src/varlinkctl/varlinkctl.c +++ b/src/varlinkctl/varlinkctl.c @@ -475,6 +475,9 @@ static int verb_call(int argc, char *argv[], void *userdata) { if (r < 0) return log_error_errno(r, "Failed to parse parameters at :%u:%u: %m", line, column); } else { + if (isatty(STDIN_FILENO) > 0 && !arg_quiet) + log_notice("Expecting method call parameter JSON object on standard input."); + r = sd_json_parse_file_at(stdin, AT_FDCWD, "", 0, &jp, &line, &column); if (r < 0) return log_error_errno(r, "Failed to parse parameters at :%u:%u: %m", line, column); From aa413d5fc6030dd09f110dad49ec972e09d45e8d Mon Sep 17 00:00:00 2001 From: Lennart Poettering Date: Mon, 27 May 2024 17:40:35 +0200 Subject: [PATCH 4/5] ci: add superficial --quiet testcase --- test/units/TEST-74-AUX-UTILS.varlinkctl.sh | 2 ++ 1 file changed, 2 insertions(+) diff --git a/test/units/TEST-74-AUX-UTILS.varlinkctl.sh b/test/units/TEST-74-AUX-UTILS.varlinkctl.sh index 9de014bdad..030f077c85 100755 --- a/test/units/TEST-74-AUX-UTILS.varlinkctl.sh +++ b/test/units/TEST-74-AUX-UTILS.varlinkctl.sh @@ -40,9 +40,11 @@ varlinkctl introspect -j /run/systemd/journal/io.systemd.journal io.systemd.Jour if command -v userdbctl >/dev/null; then systemctl start systemd-userdbd varlinkctl call /run/systemd/userdb/io.systemd.Multiplexer io.systemd.UserDatabase.GetUserRecord '{ "userName" : "testuser", "service" : "io.systemd.Multiplexer" }' + varlinkctl call -q /run/systemd/userdb/io.systemd.Multiplexer io.systemd.UserDatabase.GetUserRecord '{ "userName" : "testuser", "service" : "io.systemd.Multiplexer" }' varlinkctl call -j /run/systemd/userdb/io.systemd.Multiplexer io.systemd.UserDatabase.GetUserRecord '{ "userName" : "testuser", "service" : "io.systemd.Multiplexer" }' | jq . # We ignore the return value of the following two calls, since if no memberships are defined at all this will return a NotFound error, which is OK (varlinkctl call --more /run/systemd/userdb/io.systemd.Multiplexer io.systemd.UserDatabase.GetMemberships '{ "service" : "io.systemd.Multiplexer" }' ||:) + (varlinkctl call --quiet --more /run/systemd/userdb/io.systemd.Multiplexer io.systemd.UserDatabase.GetMemberships '{ "service" : "io.systemd.Multiplexer" }' ||:) (varlinkctl call --more -j /run/systemd/userdb/io.systemd.Multiplexer io.systemd.UserDatabase.GetMemberships '{ "service" : "io.systemd.Multiplexer" }' ||:) | jq --seq . varlinkctl call --oneway /run/systemd/userdb/io.systemd.Multiplexer io.systemd.UserDatabase.GetMemberships '{ "service" : "io.systemd.Multiplexer" }' (! varlinkctl call --oneway /run/systemd/userdb/io.systemd.Multiplexer io.systemd.UserDatabase.GetMemberships '{ "service" : "io.systemd.Multiplexer" }' | grep .) From 710ade3a8415481757477e84382d843108ec204c Mon Sep 17 00:00:00 2001 From: Lennart Poettering Date: Mon, 27 May 2024 11:37:33 +0200 Subject: [PATCH 5/5] varlinkctl: as convencience to users, accept empty string in place of {} for empty parameter list It makes it a bit easier to use "varlinkctl call", since you can just hit ^D to call a function without params, instead of spelling out {}. --- src/varlinkctl/varlinkctl.c | 23 +++++++++++++++-------- 1 file changed, 15 insertions(+), 8 deletions(-) diff --git a/src/varlinkctl/varlinkctl.c b/src/varlinkctl/varlinkctl.c index a994c640ca..fb2c3876a6 100644 --- a/src/varlinkctl/varlinkctl.c +++ b/src/varlinkctl/varlinkctl.c @@ -451,7 +451,7 @@ static int reply_callback( static int verb_call(int argc, char *argv[], void *userdata) { _cleanup_(sd_json_variant_unrefp) sd_json_variant *jp = NULL; _cleanup_(varlink_unrefp) Varlink *vl = NULL; - const char *url, *method, *parameter; + const char *url, *method, *parameter, *source; unsigned line = 0, column = 0; int r; @@ -470,18 +470,25 @@ static int verb_call(int argc, char *argv[], void *userdata) { arg_json_format_flags |= SD_JSON_FORMAT_NEWLINE; if (parameter) { + source = ""; + /* is correct, as dispatch_verb() shifts arguments by one for the verb. */ - r = sd_json_parse_with_source(parameter, "", 0, &jp, &line, &column); - if (r < 0) - return log_error_errno(r, "Failed to parse parameters at :%u:%u: %m", line, column); + r = sd_json_parse_with_source(parameter, source, 0, &jp, &line, &column); } else { if (isatty(STDIN_FILENO) > 0 && !arg_quiet) - log_notice("Expecting method call parameter JSON object on standard input."); + log_notice("Expecting method call parameter JSON object on standard input. (Provide empty string or {} for no parameters.)"); - r = sd_json_parse_file_at(stdin, AT_FDCWD, "", 0, &jp, &line, &column); - if (r < 0) - return log_error_errno(r, "Failed to parse parameters at :%u:%u: %m", line, column); + source = ""; + + r = sd_json_parse_file_at(stdin, AT_FDCWD, source, 0, &jp, &line, &column); } + if (r < 0 && r != -ENODATA) + return log_error_errno(r, "Failed to parse parameters at %s:%u:%u: %m", source, line, column); + + /* If parsing resulted in ENODATA the provided string was empty. As convenience to users we'll accept + * that and treat it as equivalent to an empty object: as a call with empty set of parameters. This + * mirrors how we do this in our C APIs too, where we are happy to accept NULL instead of a proper + * JsonVariant object for method calls. */ r = varlink_connect_auto(&vl, url); if (r < 0)