On Tue, Oct 04, 2022 at 08:11:46PM +0000, Klemens Nanni 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):
Other function, same stuff, except here the `>/dev/null 2>&1' hammer is
required to silence the ls(1) test.
The make_dev() call is no longer silenced now but does not print on
stdout anyway; if making the device fails we'd like to know.
Otherwise if probing the disk fails it continues to be silenced.
|@@ -2311,3 +2311,2 @@ is_rootdisk() {
|
|- (
| make_dev $_d
|@@ -2322,6 +2321,6 @@ is_rootdisk() {
| umount /mnt
|- fi
|+ fi >/dev/null 2>&1
| rm -f /dev/{r,}$_d?
|+
| return $_rc
|- ) >/dev/null 2>&1
| }
Feedback? OK?
Index: install.sub
===================================================================
RCS file: /cvs/src/distrib/miniroot/install.sub,v
retrieving revision 1.1210
diff -u -p -r1.1210 install.sub
--- install.sub 5 Oct 2022 19:30:47 -0000 1.1210
+++ install.sub 9 Oct 2022 23:08:17 -0000
@@ -2309,21 +2309,20 @@ set_timezone() {
is_rootdisk() {
local _d=$1 _rc=1
- (
- make_dev $_d
- if disklabel $_d | grep -q '^ a: .*4\.2BSD ' &&
- mount -t ffs -r /dev/${_d}a /mnt; then
- if $UU; then
- ls -d
/mnt/{auto_upgrade.conf,bin,dev,etc,home,sbin,tmp,usr,var}
- else
- ls -d /mnt/{bin,dev,etc,home,sbin,tmp,usr,var}
- fi
- _rc=$?
- umount /mnt
+ make_dev $_d
+ if disklabel $_d | grep -q '^ a: .*4\.2BSD ' &&
+ mount -t ffs -r /dev/${_d}a /mnt; then
+ if $UU; then
+ ls -d
/mnt/{auto_upgrade.conf,bin,dev,etc,home,sbin,tmp,usr,var}
+ else
+ ls -d /mnt/{bin,dev,etc,home,sbin,tmp,usr,var}
fi
- rm -f /dev/{r,}$_d?
- return $_rc
- ) >/dev/null 2>&1
+ _rc=$?
+ umount /mnt
+ fi >/dev/null 2>&1
+ rm -f /dev/{r,}$_d?
+
+ return $_rc
}
# Get global root information. ie. ROOTDISK, ROOTDEV and SWAPDEV.
