Hey Lennart, Lennart Poettering [2015-01-27 23:09 +0100]: > Hmm, we already had code that checks this in place, didn#t we?
We only previously had this .sh stripping in sysv_translate_name(), but that translates and init script name like "foo.sh" to "foo.service". But that's not what we need in sysv_translate_facility() to filter out the Provides: item which refers to itself. > I mean sysv_translate_facility() already filters out the case where > the service name is identical to the provided name. Hence, why do you > need a second check for this? It's not really a new check, we just fixed the existing one: - } else if (filename && streq(name, filename)) + } else if (filename && streq(name, filename_no_sh)) I. e. if filename ends with ".sh", the streq wouldn't previously match as Provides: names don't have the ".sh" suffixes. I. e. this is the case where you encounter the item in Provides: which refers to its own name, not to other scripts. We should not create an alias symlink for that, as it would refer to itself. > I wonder if the simple fix could just be to change this: > > } else if (filename && streq(name, filename)) > > to this > > } else if (filename && streq(n, filename)) > > Or so, in that function? That particularly wouldn't help, as both "n" and "name" are "foo" here in that context. I. e. n and name are both the value of the currently processed "Provides:" list item (just that n strips off $, but that's irrelevant here). But "filename" has the .sh suffix while "Provides:" names never have .sh suffixes, so that comparison fails and without the patch it would go on and create a "foo.service -> foo.service" alias symlink. This case has been mitigated in two other places, so you need to revert both 77354c7e and the extra sanity check in add_alias() of commit 29e0e6d to see it again. So you could say that we don't need the filename_no_sh comparison in sysv_translate_facility() as the new check in add_alias() as well as 29e0e6d will save us too, but this would still feel unclean to me. Thanks, Martin -- Martin Pitt | http://www.piware.de Ubuntu Developer (www.ubuntu.com) | Debian Developer (www.debian.org) _______________________________________________ systemd-devel mailing list [email protected] http://lists.freedesktop.org/mailman/listinfo/systemd-devel
