[systemd-devel] [RFC] [PATCH] Follow symlinks in /lib

[Lennart Poettering]

On Mon, 03.03.14 10:09, Michael Stapelberg (stapelberg at debian.org) wrote:

Heya,

Hmm, I think I never reviewed this patch. Sorry for the delay! It
sometimes takes a while for me to review something, but this one is
baaaaaaaad... Sorry for that.
>  
> +#define FOLLOW_MAX 8
> +
>  static int unit_file_search(
>                  InstallContext *c,
>                  InstallInfo *info,
> @@ -1053,11 +1055,44 @@ static int unit_file_search(
>                  if (!path)
>                          return -ENOMEM;
>  
> +                int cnt = 0;
> +                for (;;) {
> +                        if (cnt++ >= FOLLOW_MAX)
> +                                return -ELOOP;
> +
> +                        r = unit_file_load(c, info, path, allow_symlink);
> +
> +                        /* symlinks are always allowed for units in {/usr,}/lib/systemd so that
> +                         * one can alias units without using Alias= (the downside of Alias= is
> +                         * that the alias only exists when the unit is enabled). */
> +                        if (r >= 0)
> +                                break;
> +
> +                        if (r != -ELOOP)
> +                                break;
> +
> +                        if (allow_symlink)
> +                                break;
> +
> +                        if (!path_startswith(path, "/lib/systemd") &&
> +                            !path_startswith(path, "/usr/lib/systemd"))
> +                                break;
> +
> +                        char *target;

Even though C99 allows declaring variables in the middle of blocks, we
don't use that, and keep code and declarations separate. 

> +                        r = readlink_and_make_absolute(path, &target);
> +                        if (r < 0)
> +                                return r;
> +                        free(path);
> +                        path = target;
> +                }
> +
>                  r = unit_file_load(c, info, path, allow_symlink);

I'd prefer if this loop would be moved into a new function
unit_file_load_follow() or so, that is basically little more than a
wrapper around unit_file_load().

Otherwise looks pretty OK.

Lennart

-- 
Lennart Poettering, Red Hat