[systemd-devel] Bug#727708: [PATCH] systemctl: allow globbing in commands which take multiple unit names

[Russ Allbery]

Zbigniew Jędrzejewski-Szmek <zbyszek at in.waw.pl> writes:

> As discussed on IRC, here's a patch which takes the simple approach of
> allowing globbing on loaded unit names in various (almost all :)) systemctl
> commands. Comments?

I don't know the coding style of systemd, so please feel free to ignore
this comment if it's not consistent with the style used elsewhere.  But
since I saw this in passing:

> diff --git a/src/core/device.c b/src/core/device.c
> index 2c44dec..d3976c9 100644
> --- a/src/core/device.c
> +++ b/src/core/device.c
> @@ -268,7 +268,7 @@ static int device_update_unit(Manager *m, struct udev_device *dev, const char *p
>                                  memcpy(e, w, l);
>                                  e[l] = 0;
>  
> -                                n = unit_name_mangle(e);
> +                                n = unit_name_mangle(e, false);
>                                  if (!n) {
>                                          r = -ENOMEM;
>                                          goto fail;

A wise person convinced me a while back to avoid boolean arguments to C
functions in most cases because the meaning of the argument is very
inobvious at the call site.  "false" what?  What's disabled?  One has to
go read the function definition to know, and while that's easy to find,
particularly with a cross-referencing editor, it takes one more step.

What I've switched to instead is using tiny enums for this purpose.  So:

    enum mangle_type {
        MANGLE_NOGLOB = 0,
        MANGLE_GLOB   = 1
    };

and then at the call site:

    n = unit_name_mangle(e, MANGLE_NOGLOB);

which makes the meaning of that argument immediately obvious.

-- 
Russ Allbery (rra at debian.org)               <http://www.eyrie.org/~eagle/>


--
To UNSUBSCRIBE, email to debian-bugs-dist-REQUEST at lists.debian.org
with a subject of "unsubscribe". Trouble? Contact listmaster at lists.debian.org