On October 4, 2022 10:11:46 PM GMT+02:00, Klemens Nanni <[email protected]>
wrote:
>This function's style is a bit off: it wraps the body in a subshell to
>discard all stdout/err at once, but a 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
> | }
>
>Feedback? OK?
While I dislike the ">/dev/null 2>&1" sledgehammer, this is in the right
direction.
ok halex@
>
>
>Index: install.sub
>===================================================================
>RCS file: /cvs/src/distrib/miniroot/install.sub,v
>retrieving revision 1.1209
>diff -u -p -r1.1209 install.sub
>--- install.sub 4 Oct 2022 19:59:10 -0000 1.1209
>+++ install.sub 4 Oct 2022 20:00:54 -0000
>@@ -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))
>
