[systemd-devel] [PATCH/resend] systemctl: check the argument to -t for invalid values

[Lennart Poettering]

On Wed, 06.06.12 12:07, Zbigniew Jędrzejewski-Szmek (zbyszek at in.waw.pl) wrote:

> Systemctl accepted anything as the argument for -t, and simply said '0
> units found'. It is better to catch this user error early.

heya!

sorry for the really late review!

I like the idea, but a few comments.

> --- a/src/systemctl/systemctl.c
> +++ b/src/systemctl/systemctl.c
> @@ -4325,6 +4325,19 @@ static int runlevel_help(void) {
>          return 0;
>  }
>  +static int arg_type_ok(const char *type) {
> +
> +        static const char *types[] = {"service", "socket", "device",
> "mount",
> +                                      "swap", "automount", "target",
> "path",
> +                                      "timer", "snapshot"};
> +        unsigned i;
> +
> +        for (i=0; i<ELEMENTSOF(types); i++)
> +                if (!strcmp(type, types[i]))
> +                        return true;
> +        return false;
> +}


I think this function would be much better placed in unit-name.[ch], and
a generic name "unit_type_valid() or so. Also, please use a single
string for the list, and traverse it with NULSTR_FOREACH.

Otherwise looks good.

Lennart

-- 
Lennart Poettering - Red Hat, Inc.