Get rid of useless/confusing subshell
authorkn <kn@openbsd.org>
Wed, 5 Oct 2022 19:30:47 +0000 (19:30 +0000)
committerkn <kn@openbsd.org>
Wed, 5 Oct 2022 19:30:47 +0000 (19:30 +0000)
This function's style is a bit off:  it wraps the body in a subshell to
discard all stdout/err at once, but still uses return inside it.

1. A command list (using {}) would be enough here as it groups like a
   subshell but avoids spawning another shell;
2. discarding stdout/err at the end of an if block works the same
   (effecting both condition and body) and saves one level of indent;
3. return inside a subshell inside a function does NOT return from the
   function but merely exits the subshell;  this is easily misread.

Saving a fork and indent and improving readability boils down to this
(cvs diff -wU1):

|@@ -3320,3 +3317,2 @@ check_unattendedupgrade() {
|  _d=${_d%% *}
|- (
|  if [[ -n $_d ]]; then
|@@ -3331,5 +3327,5 @@ check_unattendedupgrade() {
|  rm -f /dev/{r,}$_d?
|- fi
|+ fi >/dev/null 2>&1
|+
|  return $_rc
|- ) > /dev/null 2>&1
| }

OK halex

distrib/miniroot/install.sub

index a93aa57..e7ae9f4 100644 (file)
@@ -1,5 +1,5 @@
 #!/bin/ksh
-#      $OpenBSD: install.sub,v 1.1209 2022/10/04 19:59:10 kn Exp $
+#      $OpenBSD: install.sub,v 1.1210 2022/10/05 19:30:47 kn Exp $
 #
 # Copyright (c) 1997-2015 Todd Miller, Theo de Raadt, Ken Westerback
 # Copyright (c) 2015, Robert Peichaer <rpe@openbsd.org>
@@ -3315,20 +3315,19 @@ check_unattendedupgrade() {
        local _d=$(get_dkdevs_root) _rc=1
 
        _d=${_d%% *}
-       (
-               if [[ -n $_d ]]; then
-                       make_dev $_d
-                       if mount -t ffs -r /dev/${_d}a /mnt; then
-                               [[ -f /mnt/bsd.upgrade && -f /mnt/auto_upgrade.conf ]]
-                               _rc=$?
-                               ((_rc == 0)) && cp /mnt/auto_upgrade.conf /
-                               echo "Which disk is the root disk = ${_d}" >> /auto_upgrade.conf
-                               umount /mnt
-                       fi
-                       rm -f /dev/{r,}$_d?
+       if [[ -n $_d ]]; then
+               make_dev $_d
+               if mount -t ffs -r /dev/${_d}a /mnt; then
+                       [[ -f /mnt/bsd.upgrade && -f /mnt/auto_upgrade.conf ]]
+                       _rc=$?
+                       ((_rc == 0)) && cp /mnt/auto_upgrade.conf /
+                       echo "Which disk is the root disk = ${_d}" >> /auto_upgrade.conf
+                       umount /mnt
                fi
-               return $_rc
-       ) > /dev/null 2>&1
+               rm -f /dev/{r,}$_d?
+       fi >/dev/null 2>&1
+
+       return $_rc
 }
 
 WATCHDOG_PERIOD_SEC=$((30 * 60))