[systemd-devel] [RFC] xyzctl-common

[Peeters Simon]

2013/11/16 Zbigniew Jędrzejewski-Szmek <zbyszek at 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.