2013/11/16 Zbigniew Jędrzejewski-Szmek <zbys...@in.waw.pl>: > On Sat, Nov 16, 2013 at 07:41:13PM +0100, Peeters Simon 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: >> >> ... >> static void help(void ) {...} >> >> enum { >> ARG_TRANSIENT = 0x100, >> ARG_STATIC, >> ARG_PRETTY, >> }; >> >> static int parse_arg(int option, const char* arg) { >> assert(option >= 0); >> >> switch (option) { >> case ARG_TRANSIENT: >> arg_transient = true; >> break; >> >> case ARG_PRETTY: >> arg_pretty = true; >> break; >> >> case ARG_STATIC: >> arg_static = true; >> break; >> >> default: >> return -EINVAL; >> } >> >> return 1; >> } >> >> int main(int argc, char *argv[]) { >> >> static const struct option options[] = { >> { "transient", no_argument, NULL, ARG_TRANSIENT }, >> { "static", no_argument, NULL, ARG_STATIC }, >> { "pretty", no_argument, NULL, ARG_PRETTY }, >> {} >> }; >> static const xyzctl_verb verbs[] = { >> { "status", LESS, 1, show_status, XYZCTL_USE_PAGER >> }, >> { "set-hostname", EQUAL, 2, set_hostname, >> XYZCTL_USE_POLKIT }, >> { "set-icon-name", EQUAL, 2, set_icon_name, >> XYZCTL_USE_POLKIT }, >> { "set-chassis", EQUAL, 2, set_chassis, >> XYZCTL_USE_POLKIT }, >> {} >> }; >> static const xyzctl_tool hostnamectl = { >> &help, >> &parse_arg, >> XYZCTL_NO_USER, >> options, >> verbs >> }; >> >> return xyzctl_main(&hostnamectl, argc, argv); >> } >> >> it would use or-able flags to controll common functionality, i currently >> have: >> enum verb_flags { >> XYZCTL_NO_BUS = 1, >> The execution of the verb does not require a bus connection. >> The system will try to connect to the bus, but will ignore any errors, >> and pass a NULL bus. >> XYZCTL_USE_PAGER = 2, >> Open a pager unless the user passes --no-pager >> XYZCTL_USE_POLKIT = 4, >> Starts the polkit agent unless the user passes --no-ask-password >> XYZCTL_LOOP_ARGS = 8, >> Executes the verb once for each argument. >> This is usefull where >> $ xyzctl verb a b c >> is the same as >> $ xyzctl verb a >> $ xyzctl verb b >> $ xyzctl verb c > I think that we do some sorting on the arguments and other operations > which mean that the behaviour for multiple args isn't usually > *exactly* the same as for one repeated. What exactly does this > flag mean?
what it does exactly: instead of calling verb.dispatch(bus, argv_left, argc_left) it does: for (int i = 1; i < argc_left; i++) verb.dispatch(bus, (char *[]){argv_left[0], argv_left[i]}, 2); an example use for it would be loginctl kill-session In the beginning I tought this would be nice to have, but I am fine with dropping this. >> }; >> >> 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 >> XYZCTL_CONNECT_SYSTEMD = 8, >> connect directly to systemd's private socket. >> } >> >> >> As a proof of concept I ported hostnemctl and loginctl resulting in +- >> 360 lines less code >> >> >> Does anybody have any comments on the basic idea and/or the proposed >> interface? > > If it makes things shorter, without making them much more complicated, > then it's probably a good idea. Can you post the full patch for > loginctl and hostnamectl? I will do this tomorow. on the calling side things get a lot simpler, the other side (the code in xyzctl-common.c) is slightly more complex because it has to dynamicly construct the options array for getopt_long, but I think it is within reason. _______________________________________________ systemd-devel mailing list systemd-devel@lists.freedesktop.org http://lists.freedesktop.org/mailman/listinfo/systemd-devel