diff mbox series

[v2,2/6] package/busybox: tidy up S01syslogd init script

Message ID 20240712124956.3925574-3-fiona.klute@gmx.de
State Accepted
Headers show
Series Update init script style | expand

Commit Message

Fiona Klute July 12, 2024, 12:49 p.m. UTC 
From: "Fiona Klute (WIWA)" <fiona.klute@gmx.de>

The manual refers to this script as a reference of how init scripts
should be written. The changes are:

* Use long form options for start-stop-daemon for clarity
* Use --exec on stop to ensure the right process gets stopped
* Avoid --quiet for clearer messages on failure
* Wait for the process to be gone during stop
* Avoid fixed wait between start and stop on restart

Signed-off-by: Fiona Klute (WIWA) <fiona.klute@gmx.de>
---
Changes v1 -> v2:
* Use start-stop-daemon --stop --test to check if the service is still
  running during stop, instead of poking at procfs

 package/busybox/S01syslogd | 18 +++++++++++++-----
 1 file changed, 13 insertions(+), 5 deletions(-)

--
2.45.2

Comments

Thomas Petazzoni July 14, 2024, 8:29 p.m. UTC | #1 
Hello Fiona,

On Fri, 12 Jul 2024 14:49:52 +0200
Fiona Klute via buildroot <buildroot@buildroot.org> wrote:

> From: "Fiona Klute (WIWA)" <fiona.klute@gmx.de>
> 
> The manual refers to this script as a reference of how init scripts
> should be written. The changes are:
> 
> * Use long form options for start-stop-daemon for clarity
> * Use --exec on stop to ensure the right process gets stopped
> * Avoid --quiet for clearer messages on failure
> * Wait for the process to be gone during stop
> * Avoid fixed wait between start and stop on restart
> 
> Signed-off-by: Fiona Klute (WIWA) <fiona.klute@gmx.de>

Applied, with one change. See below.

> +	# Wait for process to be gone, using a loop and --stop --test
> +	# because Busybox' start-stop-daemon does not support --retry
> +	# (as of 1.36.1).

I dropped this comment, which isn't really useful, because it basically
repeats what the code is doing. The fact that we can't do X or Y or Z
isn't very useful, and I didn't want to see this comment duplicated
into each and every init script.

In fact if you want to include this information, I believe the
Buildroot manual would be a better place, in the section where we
describe a canonical init script.

Best regards,

Thomas
diff mbox series

Patch

diff --git a/package/busybox/S01syslogd b/package/busybox/S01syslogd
index 15006bc06f..d3530e1f14 100644
--- a/package/busybox/S01syslogd
+++ b/package/busybox/S01syslogd
@@ -9,11 +9,12 @@  SYSLOGD_ARGS=""
 [ -r "/etc/default/$DAEMON" ] && . "/etc/default/$DAEMON"

 # BusyBox' syslogd does not create a pidfile, so pass "-n" in the command line
-# and use "-m" to instruct start-stop-daemon to create one.
+# and use "--make-pidfile" to instruct start-stop-daemon to create one.
 start() {
 	printf 'Starting %s: ' "$DAEMON"
 	# shellcheck disable=SC2086 # we need the word splitting
-	start-stop-daemon -b -m -S -q -p "$PIDFILE" -x "/sbin/$DAEMON" \
+	start-stop-daemon --start --background --make-pidfile \
+		--pidfile "$PIDFILE" --exec "/sbin/$DAEMON" \
 		-- -n $SYSLOGD_ARGS
 	status=$?
 	if [ "$status" -eq 0 ]; then
@@ -26,20 +27,27 @@  start() {

 stop() {
 	printf 'Stopping %s: ' "$DAEMON"
-	start-stop-daemon -K -q -p "$PIDFILE"
+	start-stop-daemon --stop --pidfile "$PIDFILE" --exec "/sbin/$DAEMON"
 	status=$?
 	if [ "$status" -eq 0 ]; then
-		rm -f "$PIDFILE"
 		echo "OK"
 	else
 		echo "FAIL"
+		return "$status"
 	fi
+	# Wait for process to be gone, using a loop and --stop --test
+	# because Busybox' start-stop-daemon does not support --retry
+	# (as of 1.36.1).
+	while start-stop-daemon --stop --test --quiet --pidfile "$PIDFILE" \
+		--exec "/sbin/$DAEMON"; do
+		sleep 0.1
+	done
+	rm -f "$PIDFILE"
 	return "$status"
 }

 restart() {
 	stop
-	sleep 1
 	start
 }