From: Klemens Nanni <kn@openbsd.org>
Subject: installer: move code into new stop_watchdog()
To: tech@openbsd.org
Date: Wed, 13 Mar 2024 19:35:36 +0000

We have {reset,start}_watchdog() which are only used in unattended upgrade
code, but stopping the background timer is done inline for all upgrades,
incl. interactive ones.

Below would relocate the code without logical change, but makes it obvious
how start/stop semantics are off:

| case $MODE in
| install)	do_install;;
|-upgrade)	do_upgrade;;
|+upgrade)	do_upgrade
|+		stop_watchdog;;
| esac

So only stop if UU=true, exactly like start_watchdog() is called in the
if/else block further up besaid case block.

Then start/stop code fits on one screen, is strictly limited to unattended
upgrades and hopefully more obviously so.

Feedback? Objection? OK?


Index: install.sub
===================================================================
RCS file: /cvs/src/distrib/miniroot/install.sub,v
diff -u -p -r1.1261 install.sub
--- install.sub	5 Mar 2024 19:47:58 -0000	1.1261
+++ install.sub	13 Mar 2024 19:11:29 -0000
@@ -3477,11 +3477,6 @@ do_upgrade() {
 
 	# Perform final steps common to both an install and an upgrade.
 	finish_up
-	if [ -f /tmp/wdpid ]; then
-		kill -KILL "$(</tmp/wdpid)" 2>/dev/null
-		# do not bother waiting
-		rm -f /tmp/wdpid
-	fi
 }
 
 check_unattendedupgrade() {
@@ -3528,6 +3523,15 @@ start_watchdog() {
 	set +m
 }
 
+# Stop the background timer.
+stop_watchdog() {
+	if [ -f /tmp/wdpid ]; then
+		kill -KILL "$(</tmp/wdpid)" 2>/dev/null
+		# do not bother waiting
+		rm -f /tmp/wdpid
+	fi
+}
+
 # return if we only want internal functions
 [[ -n $FUNCS_ONLY ]] && return
 
@@ -3707,6 +3711,8 @@ case $MODE in
 install)	do_install;;
 upgrade)	do_upgrade;;
 esac
+
+$UU && stop_watchdog
 
 # In case of autoinstall, this is a second process of install.sub.
 # Exiting here returns to the original process, which handles the