Hi Lennart,
Lennart Poettering <lenn...@poettering.net> writes:
> I'd prefer if this complex check (at least the last part of it) could be
> moved into its own function, to make this easier to read.
Done.
> We tend to write:
>
> if (...)
> foo;
> else
> bar;
Also done.
I’ve attached a git format-patch against the current systemd HEAD (for
merging it upstream) and an updated version of the patch for systemd 44
(for Debian).
--
Best regards,
Michael
--- systemd-44.orig/src/core/service.c 2012-10-15 00:31:34.000000000 +0200
+++ systemd-44/src/core/service.c 2012-10-16 10:06:46.676103609 +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,13 +508,24 @@
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;
}
+static bool usage_contains_reload(const char *line) {
+ return (strcasestr(line, "{reload|") ||
+ strcasestr(line, "{reload}") ||
+ strcasestr(line, "{reload\"") ||
+ strcasestr(line, "|reload|") ||
+ strcasestr(line, "|reload}") ||
+ strcasestr(line, "|reload\""));
+}
+
static int service_load_sysv_path(Service *s, const char *path) {
FILE *f;
Unit *u;
@@ -524,10 +535,12 @@
NORMAL,
DESCRIPTION,
LSB,
- LSB_DESCRIPTION
+ LSB_DESCRIPTION,
+ USAGE_CONTINUATION
} 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 +587,23 @@
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 ( state == USAGE_CONTINUATION ||
+ (state == NORMAL && strcasestr(t, "usage"))) {
+ if (usage_contains_reload(t)) {
+ supports_reload = true;
+ state = NORMAL;
+ } else if (t[strlen(t)-1] == '\\')
+ state = USAGE_CONTINUATION;
+ else
+ state = NORMAL;
+ }
+
continue;
+ }
if (state == NORMAL && streq(t, "### BEGIN INIT INFO")) {
state = LSB;
@@ -868,7 +896,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) &&
>From 72596a6c82d8ce0e17098d03924af432cc9b1e6e Mon Sep 17 00:00:00 2001
From: Michael Stapelberg <mich...@stapelberg.de>
Date: Tue, 16 Oct 2012 10:14:03 +0200
Subject: [PATCH] Heuristically determine whether SysV scripts support reload
This commit checks for a usage line which contains [{|]reload[|}"] (to
not errnously match force-reload).
---
src/core/service.c | 44 ++++++++++++++++++++++++++++++++++++--------
1 file changed, 36 insertions(+), 8 deletions(-)
diff --git a/src/core/service.c b/src/core/service.c
index 69c7150..7f9a53a 100644
--- a/src/core/service.c
+++ b/src/core/service.c
@@ -526,7 +526,7 @@ static ExecCommand *exec_command_new(const char *path, const char *arg1) {
return c;
}
-static int sysv_exec_commands(Service *s) {
+static int sysv_exec_commands(Service *s, const bool supports_reload) {
ExecCommand *c;
assert(s);
@@ -543,14 +543,25 @@ static int sysv_exec_commands(Service *s) {
return -ENOMEM;
exec_command_append_list(s->exec_command+SERVICE_EXEC_STOP, c);
- c = exec_command_new(UNIT(s)->source_path, "reload");
- if (!c)
- return -ENOMEM;
- exec_command_append_list(s->exec_command+SERVICE_EXEC_RELOAD, c);
+ if (supports_reload) {
+ c = exec_command_new(UNIT(s)->source_path, "reload");
+ if (!c)
+ return -ENOMEM;
+ exec_command_append_list(s->exec_command+SERVICE_EXEC_RELOAD, c);
+ }
return 0;
}
+static bool usage_contains_reload(const char *line) {
+ return (strcasestr(line, "{reload|") ||
+ strcasestr(line, "{reload}") ||
+ strcasestr(line, "{reload\"") ||
+ strcasestr(line, "|reload|") ||
+ strcasestr(line, "|reload}") ||
+ strcasestr(line, "|reload\""));
+}
+
static int service_load_sysv_path(Service *s, const char *path) {
FILE *f;
Unit *u;
@@ -560,10 +571,12 @@ static int service_load_sysv_path(Service *s, const char *path) {
NORMAL,
DESCRIPTION,
LSB,
- LSB_DESCRIPTION
+ LSB_DESCRIPTION,
+ USAGE_CONTINUATION
} state = NORMAL;
char *short_description = NULL, *long_description = NULL, *chkconfig_description = NULL, *description;
struct stat st;
+ bool supports_reload = false;
assert(s);
assert(path);
@@ -612,8 +625,23 @@ static int service_load_sysv_path(Service *s, const char *path) {
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 ( state == USAGE_CONTINUATION ||
+ (state == NORMAL && strcasestr(t, "usage"))) {
+ if (usage_contains_reload(t)) {
+ supports_reload = true;
+ state = NORMAL;
+ } else if (t[strlen(t)-1] == '\\')
+ state = USAGE_CONTINUATION;
+ else
+ state = NORMAL;
+ }
+
continue;
+ }
if (state == NORMAL && streq(t, "### BEGIN INIT INFO")) {
state = LSB;
@@ -906,7 +934,7 @@ static int service_load_sysv_path(Service *s, const char *path) {
}
}
- 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) &&
--
1.7.10.4
