On Sat, 16.11.13 19:41, Peeters Simon (peeters.si...@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 _______________________________________________ systemd-devel mailing list systemd-devel@lists.freedesktop.org http://lists.freedesktop.org/mailman/listinfo/systemd-devel