[systemd-devel] [PATCH] systemctl: clean up output a little bit

[Lennart Poettering]

On Tue, 31.03.15 11:52, Shawn Landden (shawn at churchofgit.com) wrote:

> ---
>  src/systemctl/systemctl.c | 51 +++++++++++++++++++++++++++++++++++++++++++++++
>  1 file changed, 51 insertions(+)
> 
> diff --git a/src/systemctl/systemctl.c b/src/systemctl/systemctl.c
> index 3158a38..bf279f9 100644
> --- a/src/systemctl/systemctl.c
> +++ b/src/systemctl/systemctl.c
> @@ -4133,6 +4133,57 @@ static int print_property(const char *name, sd_bus_message *m, const char *conte
>                  }
>  
>                  break;
> +
> +        case SD_BUS_TYPE_UINT64:
> +                   /* alternatively we could print these as "infinity" like CPUQuotaPerSecUSec */
> +                if (startswith(name, "Limit") ||
> +                    streq(name, "CPUShares") ||
> +                    streq(name, "StartupCPUShares") ||
> +                    streq(name, "BlockIOWeight") ||
> +                    streq(name, "StartupBlockIOWeight") ||
> +                    /* confusing that "MemoryCurrent" could be
> UINT64_MAX */

Hmm, what is this about?

> +                    streq(name, "MemoryCurrent") ||
> +                    streq(name, "MemoryLimit") ||
> +
> +                    streq(name, "CapabilityBoundingSet")) {
> +                        uint64_t u64;
> +
> +                        r = sd_bus_message_read_basic(m, 't', &u64);
> +                        if (r < 0)
> +                                return bus_log_parse_error(r);
> +
> +                        if ((u64 != (uint64_t) - 1) || arg_all)
> +                                printf("%s=%"PRIu64"\n", name, u64);
> +
> +                        return 0;
> +
> +                }
> +
> +                break;
> +
> +        case SD_BUS_TYPE_BOOLEAN:
> +
> +                if (endswith(name, "Accounting") ||
> +                    startswith(name, "Private") ||
> +                    streq(name, "NoNewPrivileges") ||
> +                    streq(name, "NonBlocking") ||
> +                    streq(name, "CPUSchedulingResetOnFork") ||
> +                    streq(name, "Delegate") ||
> +                    streq(name, "Transient")) {
> +                        bool b;
> +
> +                        r = sd_bus_message_read_basic(m, 'b', &b);
> +                        if (r < 0)
> +                                return bus_log_parse_error(r);
> +
> +                        if ((b == true) || arg_all)
> +                                printf("%s=%s\n", name, yes_no(b));
> +
> +                        return 0;
> +
> +                }
> +
> +                break;

No, I really don't like this. I don't want to encode tables with
default values on the client side. First of all these tables can get out of
date, and secondly, the defaults for many of these can actually be
altered via system.conf as mentioned earlier.

In fact, I am tempted to go the other way and drop the suppression of
"empty" fields, and simply always show all fields.

I'd be willing to add some extra code to bus_print_property() which
displays the special value (uint64_t) -1 as "-1" instead of
18446744073709551615. And similar for uint32_t... That should slightly
improve the output already.

Another idea to think about might be to just drop all special
formatting in the "show" output and instead use the sd-bus string
serialization that "busctl introspect" shows for all the fields. That
format is shell parsable at least, and somewhat easy to
grok. But I figure we can't really do that, since we claim that
"systemctl show" is machine parsable, and making major changes to its
formatting should hence not be OK.

Lennart

-- 
Lennart Poettering, Red Hat