Subject: bug#599: initscripts: typo in /etc/init.d/sendsigs (easy fix)

From: Jerome BENOIT <[email-masked]>
Date: Sat, 07 Aug 2021 17:23:17 +0000

Package: initscripts
Version: 2.93-8+devuan1
Severity: normal

Dear Maintainer,

	In the /etc/init.d/sendsigs script, at the beginning of function
	do_stop(), in the for-loop, the in-argument seems to be mispelled:
	the terminal '.d' is missing.

Cheers, and thanks for your work,
Jerome

-- System Information:
Distributor ID:	Devuan
Description:	Devuan GNU/Linux 3 (beowulf)
Release:	3
Codename:	beowulf
Architecture: x86_64

Kernel: Linux 5.10.0-0.bpo.7-amd64 (SMP w/24 CPU cores)
Locale: LANG=en_GB.UTF-8, LC_CTYPE=en_GB.UTF-8 (charmap=UTF-8), LANGUAGE=en_GB:en (charmap=UTF-8)
Shell: /bin/sh linked to /bin/dash
Init: sysvinit (via /sbin/init)

Versions of packages initscripts depends on:
ii  coreutils       8.30-3
ii  debianutils     4.8.6.1
ii  lsb-base        10.2019051400
ii  mount           2.33.1-0.1+devuan1~beowulf2
ii  sysv-rc         2.93-8+devuan1
ii  sysvinit-utils  2.93-8+devuan1

Versions of packages initscripts recommends:
ii  e2fsprogs  1.44.5-1+deb10u3
ii  psmisc     23.2-1

initscripts suggests no packages.

-- no debconf information

---

From: Mark Hindley <[email-masked]>
Date: Sat, 7 Aug 2021 19:23:56 +0100

Control: tags -1 moreinfo

Jerome,

Thanks for this.

> 	the terminal '.d' is missing.

Hmmm, I am not so sure about this.
 
> do_stop () {
> 	OMITPIDS=
> 
> 	for omitfile in /run/sendsigs.omit; do

I think this is where you think the typo is ^^^^?

Isn't this legacy handling for /run/sendsigs.omit? Admittedly that file is not
present on my beowulf system..

> 		if [ -e $omitfile ]; then

.. so this is skipped.

> 			for pid in $(cat $omitfile); do
> 				OMITPIDS="${OMITPIDS:+$OMITPIDS }-o $pid"
> 			done
> 		fi
> 	done
> 
> 	# Load sendsigs.omit.d/packagename files too, to make it
> 	# possible for scripts that need to modify the list of pids at
> 	# run time without race conditions.
> 	for omitdir in /run/sendsigs.omit.d; do

Files in /run/sendsigs.omit.d/ are handled here.

> 		if [ -d "${omitdir}" ]; then
> 			for pidfile in "${omitdir}/"*; do
> 				[ -f "$pidfile" ] || continue
> 				for pid in $(cat $pidfile); do
> 					OMITPIDS="${OMITPIDS:+$OMITPIDS }-o $pid"
> 				done
> 			done
> 		fi
> 	done

Or have I misunderstood you?

Thanks

Mark

---

From: Jerome BENOIT <[email-masked]>
Date: Sat, 7 Aug 2021 22:51:02 +0200

> Control: tags -1 moreinfo

Actually I think that my first fix was actually incorrect.

Please find in attachment the current version, "my" current version, and
the corresponding patch.

I think that otherwise these loops make no sense.

Cheers,
Jerome
--- sendsigs-dpkg-dist-with_typo	2019-02-26 17:59:47.000000000 +0100
+++ sendsigs	2021-08-07 22:45:52.619263748 +0200
@@ -27,8 +27,8 @@
 do_stop () {
 	OMITPIDS=
 
-	for omitfile in /run/sendsigs.omit; do
-		if [ -e $omitfile ]; then
+	for omitfile in /run/sendsigs.omit.d/*; do
+		if [ -f $omitfile ]; then
 			for pid in $(cat $omitfile); do
 				OMITPIDS="${OMITPIDS:+$OMITPIDS }-o $pid"
 			done
@@ -38,7 +38,7 @@
 	# Load sendsigs.omit.d/packagename files too, to make it
 	# possible for scripts that need to modify the list of pids at
 	# run time without race conditions.
-	for omitdir in /run/sendsigs.omit.d; do
+	for omitdir in /run/sendsigs.omit.d/*; do
 		if [ -d "${omitdir}" ]; then
 			for pidfile in "${omitdir}/"*; do
 				[ -f "$pidfile" ] || continue

---

From: Mark Hindley <[email-masked]>
Date: Sun, 8 Aug 2021 07:47:11 +0100

> I think that otherwise these loops make no sense.

I am still not convinced by this. Read /usr/share/doc/initscripts/README.Debian
to see an explanation of the intented behaviour. In particular

   Process id's listed in /run/sendsigs.omit, or any file in the
   /run/sendsigs.omit.d/ directory will be omitted by sendsigs.

I don't see any discrepancy between the existing distributed code and that
stated behaviour. What am I missing?

Thanks

Mark

---

From: Mark Hindley <[email-masked]>
Date: Sun, 8 Aug 2021 07:53:37 +0100

> stated behaviour. What am I missing?

I should have added that I agree the for loops are not strictly necessary. The
appear left over from when /lib/init/rw was also used as well as /run. See
commit a7469405b3a2d25c0f80916e912a62fcbbe433e7 for the removal of that which
still left the outer for loops intact. But I still don't see a bug.

Mark

---

From: Jerome BENOIT <[email-masked]>
Date: Sun, 8 Aug 2021 14:36:57 +0200

Hello Mark, thanks for pointing me to /usr/share/doc/initscripts/README.Debian .
I am sorry I could not find the commit you reference to.

Indeed the current script fulfills the policy in the above README.Debian .

I would however make the minor change in the first loop [ -e $omitfile ] to [ -f $omitfile ] to make clearer
that a file is expected (see patch 599_bugs_devuan_org-sendsigs-filetest.patch )

Having said that I think that making loops over one element is rather confusing
and it deserves some explanation if it must be kept it that way
(see patch 599_bugs_devuan_org-sendsigs-lessconfusing.patch ).

Cheers,
Jerome

-- 
Jerome BENOIT | calculus+at-rezozer^dot*net
https://qa.debian.org/developer.php?login=[email-masked]
AE28 AE15 710D FF1D 87E5  A762 3F92 19A6 7F36 C68B
--- sendsigs.dpkg-dist-original	2019-02-26 17:59:47.000000000 +0100
+++ sendsigs	2021-08-08 14:03:35.860634425 +0200
@@ -28,7 +28,7 @@
 	OMITPIDS=
 
 	for omitfile in /run/sendsigs.omit; do
-		if [ -e $omitfile ]; then
+		if [ -f $omitfile ]; then
 			for pid in $(cat $omitfile); do
 				OMITPIDS="${OMITPIDS:+$OMITPIDS }-o $pid"
 			done
--- sendsigs.dpkg-dist-original	2019-02-26 17:59:47.000000000 +0100
+++ sendsigs	2021-08-08 14:20:43.607818183 +0200
@@ -27,27 +27,25 @@
 do_stop () {
 	OMITPIDS=
 
-	for omitfile in /run/sendsigs.omit; do
-		if [ -e $omitfile ]; then
-			for pid in $(cat $omitfile); do
-				OMITPIDS="${OMITPIDS:+$OMITPIDS }-o $pid"
-			done
-		fi
-	done
+	omitfile="/run/sendsigs.omit"
+	if [ -f "${omitfile}" ]; then
+		for pid in $(cat "${omitfile}"); do
+			OMITPIDS="${OMITPIDS:+$OMITPIDS }-o $pid"
+		done
+	fi
 
 	# Load sendsigs.omit.d/packagename files too, to make it
 	# possible for scripts that need to modify the list of pids at
 	# run time without race conditions.
-	for omitdir in /run/sendsigs.omit.d; do
-		if [ -d "${omitdir}" ]; then
-			for pidfile in "${omitdir}/"*; do
-				[ -f "$pidfile" ] || continue
-				for pid in $(cat $pidfile); do
-					OMITPIDS="${OMITPIDS:+$OMITPIDS }-o $pid"
-				done
+	omitdir="/run/sendsigs.omit.d"
+	if [ -d "${omitdir}" ]; then
+		for pidfile in "${omitdir}/"*; do
+			[ -f "${pidfile}" ] || continue
+			for pid in $(cat "${pidfile}"); do
+				OMITPIDS="${OMITPIDS:+$OMITPIDS }-o $pid"
 			done
-		fi
-	done
+		done
+	fi
 
 	# Upstart jobs have their own "stop on" clauses that sends
 	# SIGTERM/SIGKILL just like this, so if they're still running,