From 0e6872cdfcfefcee20f566bde25a89b74e55eef3 Mon Sep 17 00:00:00 2001 From: Lennart Poettering Date: Wed, 19 Sep 2018 20:53:22 +0300 Subject: [PATCH 1/4] inhibit: normalize when we log about failures to list inhibitors let's print log messages about all types of errors inside of the function, since otherwise we might sometimes log twice about some specific cases. --- src/login/inhibit.c | 15 +++++++-------- 1 file changed, 7 insertions(+), 8 deletions(-) diff --git a/src/login/inhibit.c b/src/login/inhibit.c index 1daaa8b450..b19369aa76 100644 --- a/src/login/inhibit.c +++ b/src/login/inhibit.c @@ -60,7 +60,8 @@ static int inhibit(sd_bus *bus, sd_bus_error *error) { return r; } -static int print_inhibitors(sd_bus *bus, sd_bus_error *error) { +static int print_inhibitors(sd_bus *bus) { + _cleanup_(sd_bus_error_free) sd_bus_error error = SD_BUS_ERROR_NULL; _cleanup_(sd_bus_message_unrefp) sd_bus_message *reply = NULL; const char *what, *who, *why, *mode; unsigned int uid, pid; @@ -75,11 +76,11 @@ static int print_inhibitors(sd_bus *bus, sd_bus_error *error) { "/org/freedesktop/login1", "org.freedesktop.login1.Manager", "ListInhibitors", - error, + &error, &reply, ""); if (r < 0) - return r; + return log_error_errno(r, "Could not get active inhibitors: %s", bus_error_message(&error, r)); r = sd_bus_message_enter_container(reply, SD_BUS_TYPE_ARRAY, "(ssssuu)"); if (r < 0) @@ -227,7 +228,6 @@ static int parse_argv(int argc, char *argv[]) { } int main(int argc, char *argv[]) { - _cleanup_(sd_bus_error_free) sd_bus_error error = SD_BUS_ERROR_NULL; _cleanup_(sd_bus_flush_close_unrefp) sd_bus *bus = NULL; int r; @@ -248,14 +248,13 @@ int main(int argc, char *argv[]) { if (arg_action == ACTION_LIST) { - r = print_inhibitors(bus, &error); + r = print_inhibitors(bus); pager_close(); - if (r < 0) { - log_error("Failed to list inhibitors: %s", bus_error_message(&error, -r)); + if (r < 0) return EXIT_FAILURE; - } } else { + _cleanup_(sd_bus_error_free) sd_bus_error error = SD_BUS_ERROR_NULL; _cleanup_close_ int fd = -1; _cleanup_free_ char *w = NULL; pid_t pid; From 2f47ef04ea787ae397ba799dde6892b3e4bc9245 Mon Sep 17 00:00:00 2001 From: Lennart Poettering Date: Wed, 19 Sep 2018 20:57:42 +0300 Subject: [PATCH 2/4] inhibit: normalize variable types When we parse an "u" from an sd_bus_message then we need to do that into a uint32_t, not a pid_t or uid_t, even if this is likely the same. Also, let's count objects we keep in memory as size_t as usual. --- src/login/inhibit.c | 8 ++++---- 1 file changed, 4 insertions(+), 4 deletions(-) diff --git a/src/login/inhibit.c b/src/login/inhibit.c index b19369aa76..e01aee6a5d 100644 --- a/src/login/inhibit.c +++ b/src/login/inhibit.c @@ -64,8 +64,8 @@ static int print_inhibitors(sd_bus *bus) { _cleanup_(sd_bus_error_free) sd_bus_error error = SD_BUS_ERROR_NULL; _cleanup_(sd_bus_message_unrefp) sd_bus_message *reply = NULL; const char *what, *who, *why, *mode; - unsigned int uid, pid; - unsigned n = 0; + uint32_t uid, pid; + size_t n = 0; int r; (void) pager_open(arg_no_pager, false); @@ -95,7 +95,7 @@ static int print_inhibitors(sd_bus *bus) { get_process_comm(pid, &comm); u = uid_to_name(uid); - printf(" Who: %s (UID "UID_FMT"/%s, PID "PID_FMT"/%s)\n" + printf(" Who: %s (UID %" PRIu32 "/%s, PID %" PRIu32"/%s)\n" " What: %s\n" " Why: %s\n" " Mode: %s\n\n", @@ -113,7 +113,7 @@ static int print_inhibitors(sd_bus *bus) { if (r < 0) return bus_log_parse_error(r); - printf("%u inhibitors listed.\n", n); + printf("%zu inhibitors listed.\n", n); return 0; } From a9426617870e93765bf6cd73a7ab921fe55c267a Mon Sep 17 00:00:00 2001 From: Lennart Poettering Date: Wed, 19 Sep 2018 21:21:45 +0300 Subject: [PATCH 3/4] inhibit: use format-table to format systemd-inhibit --list This changes the output a bit, as the previous multi-line output of each inhibitor is changed to a single line, but it does unify the output look with the one of our other tools. Moreover this adds proper sorting. --- man/systemd-inhibit.xml | 1 + src/login/inhibit.c | 75 +++++++++++++++++++++++++++++++---------- 2 files changed, 58 insertions(+), 18 deletions(-) diff --git a/man/systemd-inhibit.xml b/man/systemd-inhibit.xml index 03dc06678a..3fa5acf550 100644 --- a/man/systemd-inhibit.xml +++ b/man/systemd-inhibit.xml @@ -119,6 +119,7 @@ + diff --git a/src/login/inhibit.c b/src/login/inhibit.c index e01aee6a5d..8fe64b9184 100644 --- a/src/login/inhibit.c +++ b/src/login/inhibit.c @@ -12,6 +12,7 @@ #include "bus-error.h" #include "bus-util.h" #include "fd-util.h" +#include "format-table.h" #include "format-util.h" #include "pager.h" #include "process-util.h" @@ -26,6 +27,7 @@ static const char* arg_who = NULL; static const char* arg_why = "Unknown reason"; static const char* arg_mode = NULL; static bool arg_no_pager = false; +static bool arg_legend = true; static enum { ACTION_INHIBIT, @@ -63,9 +65,7 @@ static int inhibit(sd_bus *bus, sd_bus_error *error) { static int print_inhibitors(sd_bus *bus) { _cleanup_(sd_bus_error_free) sd_bus_error error = SD_BUS_ERROR_NULL; _cleanup_(sd_bus_message_unrefp) sd_bus_message *reply = NULL; - const char *what, *who, *why, *mode; - uint32_t uid, pid; - size_t n = 0; + _cleanup_(table_unrefp) Table *table = NULL; int r; (void) pager_open(arg_no_pager, false); @@ -82,38 +82,70 @@ static int print_inhibitors(sd_bus *bus) { if (r < 0) return log_error_errno(r, "Could not get active inhibitors: %s", bus_error_message(&error, r)); + table = table_new("WHO", "UID", "USER", "PID", "COMM", "WHAT", "WHY", "MODE"); + if (!table) + return log_oom(); + + /* If there's not enough space, shorten the "WHY" column, as it's little more than an explaining comment. */ + (void) table_set_weight(table, TABLE_HEADER_CELL(6), 20); + r = sd_bus_message_enter_container(reply, SD_BUS_TYPE_ARRAY, "(ssssuu)"); if (r < 0) return bus_log_parse_error(r); - while ((r = sd_bus_message_read(reply, "(ssssuu)", &what, &who, &why, &mode, &uid, &pid)) > 0) { + for (;;) { _cleanup_free_ char *comm = NULL, *u = NULL; + const char *what, *who, *why, *mode; + uint32_t uid, pid; + + r = sd_bus_message_read(reply, "(ssssuu)", &what, &who, &why, &mode, &uid, &pid); + if (r < 0) + return bus_log_parse_error(r); + if (r == 0) + break; if (arg_mode && !streq(mode, arg_mode)) continue; - get_process_comm(pid, &comm); + (void) get_process_comm(pid, &comm); u = uid_to_name(uid); - printf(" Who: %s (UID %" PRIu32 "/%s, PID %" PRIu32"/%s)\n" - " What: %s\n" - " Why: %s\n" - " Mode: %s\n\n", - who, uid, strna(u), pid, strna(comm), - what, - why, - mode); - - n++; + r = table_add_many(table, + TABLE_STRING, who, + TABLE_UINT32, uid, + TABLE_STRING, strna(u), + TABLE_UINT32, pid, + TABLE_STRING, strna(comm), + TABLE_STRING, what, + TABLE_STRING, why, + TABLE_STRING, mode); + if (r < 0) + return log_error_errno(r, "Failed to add table row: %m"); } - if (r < 0) - return bus_log_parse_error(r); r = sd_bus_message_exit_container(reply); if (r < 0) return bus_log_parse_error(r); - printf("%zu inhibitors listed.\n", n); + if (table_get_rows(table) > 1) { + r = table_set_sort(table, (size_t) 1, (size_t) 0, (size_t) 5, (size_t) 6, (size_t) -1); + if (r < 0) + return log_error_errno(r, "Failed to sort table: %m"); + + table_set_header(table, arg_legend); + + r = table_print(table, NULL); + if (r < 0) + return log_error_errno(r, "Failed to show table: %m"); + } + + if (arg_legend) { + if (table_get_rows(table) > 1) + printf("\n%zu inhibitors listed.\n", table_get_rows(table) - 1); + else + printf("No inhibitors.\n"); + } + return 0; } @@ -130,6 +162,7 @@ static int help(void) { " -h --help Show this help\n" " --version Show package version\n" " --no-pager Do not pipe output into a pager\n" + " --no-legend Do not show the headers and footers\n" " --what=WHAT Operations to inhibit, colon separated list of:\n" " shutdown, sleep, idle, handle-power-key,\n" " handle-suspend-key, handle-hibernate-key,\n" @@ -156,6 +189,7 @@ static int parse_argv(int argc, char *argv[]) { ARG_MODE, ARG_LIST, ARG_NO_PAGER, + ARG_NO_LEGEND, }; static const struct option options[] = { @@ -167,6 +201,7 @@ static int parse_argv(int argc, char *argv[]) { { "mode", required_argument, NULL, ARG_MODE }, { "list", no_argument, NULL, ARG_LIST }, { "no-pager", no_argument, NULL, ARG_NO_PAGER }, + { "no-legend", no_argument, NULL, ARG_NO_LEGEND }, {} }; @@ -209,6 +244,10 @@ static int parse_argv(int argc, char *argv[]) { arg_no_pager = true; break; + case ARG_NO_LEGEND: + arg_legend = false; + break; + case '?': return -EINVAL; From bd1b3f75e81f418a04bc44a3c42ea3c882077b89 Mon Sep 17 00:00:00 2001 From: Lennart Poettering Date: Mon, 24 Sep 2018 19:01:08 +0200 Subject: [PATCH 4/4] update TODO --- TODO | 4 +++- 1 file changed, 3 insertions(+), 1 deletion(-) diff --git a/TODO b/TODO index 2edfc51644..d56ff94cf3 100644 --- a/TODO +++ b/TODO @@ -17,6 +17,8 @@ Janitorial Clean-ups: Features: +* chown() tty a service is attached to after the service goes down + * optionally, if a per-partition GPT flag is set for the root/home/… partitions format the partition on next boot and unset the flag, in order to implement factory reset. also, add a second flag that simply indicates whether such a @@ -110,7 +112,7 @@ Features: zero and is not open anymore, while the latter happens when a file is unlinked from any dir. -* port systemctl, systemd-inhibit, busctl, … over to format-table.[ch]'s table formatters +* port systemctl, busctl, … over to format-table.[ch]'s table formatters * pid1: lock image configured with RootDirectory=/RootImage= using the usual nspawn semantics while the unit is up