On Mon, 03.03.14 10:09, Michael Stapelberg (stapelb...@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 _______________________________________________ systemd-devel mailing list systemd-devel@lists.freedesktop.org http://lists.freedesktop.org/mailman/listinfo/systemd-devel