Hi Michael, On 14.10.2012 02:09, Michael Stapelberg wrote: > On Tue, 28 Aug 2012 20:42:52 +0200 > Michael Biebl <[email protected]> wrote: >> We could extend that and check for the "Usage" message and set the >> Reload flag depending on wether we find a "reload" string in the help >> message.
[..]
> The patch checks for '|reload[|}"]'. There is only one of 1187 init
> scripts (provided by mbiebl, thanks!) which does not match this
> expression but supports reload.
First of all, thanks for the patch.
The test-set we used were the init scripts shipped in Debian sid from a
month ago. I've updated that to the latest versions and made it
available at [1] in case anyone is interested.
Some numbers:
a/ # of initscripts: 1192 (+3 symlinks)
$ find . -type f | wc -l
1192
b/ # of initscripts with a Usage line (I additionally check for start to
limit false positives)
$ grep -vE "^\s*#.*" */*/* | grep "[Uu]sage.*start*" | wc -l
1153
c/ # of initscripts matching '|reload[|}"]':
grep -vE "^\s*#.*" */*/* | grep '[Uu]sage.*|reload[|}"]' | wc -l
351
d/ # checking for '[{|]reload[|}"]' doesn't produce additional hits:
grep -vE "^\s*#.*" */*/* | grep '[Uu]sage.*[{|]reload[|}"]' | wc -l
351
e/ # of initscripts matching '[^-]reload[^-]'
grep -vE "^\s*#.*" */*/* | grep '[Uu]sage.*[^-]reload[^-]' | wc -l
352
the additional hit compared to c/ is
am-utils_6.2+rc20110530-3/init.d/am-utils: echo "Usage: /etc/init.d/amd
{start|stop|restart|[force-]reload}"
f/ # of Usage lines using continuations:
$ grep -vE "^\s*#.*" */*/* | grep '[Uu]sage.*\\$' | wc -l
10
Comments:
1/ I think it would be nice to check for [{|]reload[}|"] as delimiters,
even if d/ currently doesn't turn up any additional hits. Just to be
future-proof.
2/ f/ makes make think we should handle line continuations. This should
probably be done like the existing code, using a STATE variable, i.e. we
enter that state once we find ".*\\$" and leave it again as soon as this
is no longer the case.
3/ As Lennart has put this issue on his upstream TODO list [2], I've
CCed him, in case he wants to share some input
Cheers,
Michael
[1] http://people.debian.org/~biebl/init-2012-10-14.tar.gz
[2] http://cgit.freedesktop.org/systemd/systemd/tree/TODO#n353
--
Why is it that all of the instruments seeking intelligent life in the
universe are pointed away from Earth?
--- systemd-44.orig/src/service.c 2012-03-12 21:49:16.000000000 +0100
+++ systemd-44/src/service.c 2012-10-14 01:48:40.268064643 +0200
@@ -494,7 +494,7 @@
return c;
}
-static int sysv_exec_commands(Service *s) {
+static int sysv_exec_commands(Service *s, const bool supports_reload) {
ExecCommand *c;
assert(s);
@@ -508,9 +508,11 @@
return -ENOMEM;
exec_command_append_list(s->exec_command+SERVICE_EXEC_STOP, c);
- if (!(c = exec_command_new(s->sysv_path, "reload")))
- return -ENOMEM;
- exec_command_append_list(s->exec_command+SERVICE_EXEC_RELOAD, c);
+ if (supports_reload) {
+ if (!(c = exec_command_new(s->sysv_path, "reload")))
+ return -ENOMEM;
+ exec_command_append_list(s->exec_command+SERVICE_EXEC_RELOAD, c);
+ }
return 0;
}
@@ -528,6 +530,7 @@
} state = NORMAL;
char *short_description = NULL, *long_description = NULL, *chkconfig_description = NULL, *description;
struct stat st;
+ bool supports_reload = false;
assert(s);
assert(path);
@@ -574,8 +577,19 @@
line++;
t = strstrip(l);
- if (*t != '#')
+ if (*t != '#') {
+ /* Try to figure out whether this init script supports
+ * the reload operation. This heuristic looks for
+ * "Usage: " lines which include the reload option. */
+ if (strcasestr(t, "usage") &&
+ (strcasestr(t, "|reload|") ||
+ strcasestr(t, "|reload}") ||
+ strcasestr(t, "|reload\""))) {
+ supports_reload = true;
+ }
+
continue;
+ }
if (state == NORMAL && streq(t, "### BEGIN INIT INFO")) {
state = LSB;
@@ -868,7 +882,7 @@
}
}
- if ((r = sysv_exec_commands(s)) < 0)
+ if ((r = sysv_exec_commands(s, supports_reload)) < 0)
goto finish;
if (s->sysv_runlevels &&
chars_intersect(RUNLEVELS_BOOT, s->sysv_runlevels) &&
signature.asc
Description: OpenPGP digital signature
