[systemd-devel] [RFC] xyzctl-common

[Lennart Poettering]

On Sat, 16.11.13 19:41, Peeters Simon (peeters.simon at gmail.com) wrote:

> hello all,
> 
> During the sd_bus porting I noted that the *ctl tools (and
> systemd-analyze) contain a lot of common boilerplate code.
> 
> So the basic idea is to split this boilerplate out into
> xyzctl-common.[ch] so that f.ex the bottom hostnamectl.c would look
> like:

There actually has been a lon-standing TODO item to unify the code that
handles the "verbs" parsing in the various tools. In fact there's
currently a bit too much copy/paste going on. For example, we have
parsing code handling this in timedatectl, localectl, loginctl,
systemctl at least. They are not identical bits of code though, but very
similar (systemctl has some special magic to print bus errors only in
some cases...). But either way: this should really be unified.

>         enum tool_flags {
>                 XYZCTL_NO_REMOTE = 1,
> Do not allow the use of --host=HOST to connect to a remote host
>                 XYZCTL_NO_MACHINE = 2,
> Do not allow the use of --machine=CONTAINER to connect to a container
>                 XYZCTL_NO_USER = 4,
> Do not allow connection to the user bus by using --user

I really don't like "negative" bools in our code. It's bad enough we
support them as command line switches, but internally we shouldn't
create more like that...

>                 XYZCTL_CONNECT_SYSTEMD = 8,
> connect directly to systemd's private socket.
>         }

Hmmm, I do like the verb_flags thing but I am not sold to the tool_flags
idea... the systemd conenction thing for example appears so specific,
that I wouldn't want to generalize that, especially since its going to
go away with kdbus.

So, I think for now, I'd just like to see unification of
loginctl_main(), timedatectl_main(), localectl_main(),
hostnamectl_main(), ... covering the verb flags, but I'd like to avoid
the tool flags for now, let's leave this explicit in the tools, we can
generalize this later on, should it really be necessary after kdbus is
a requirement and we can drop the magic dbus code...

Anyway, all work on this appreciated!

(and srry for the long delay in reviewing, drowning in patches...)

Lennart

-- 
Lennart Poettering, Red Hat