The general rule is now: Align stat_{fail,done} with stat_busy --- rc.sysinit | 2 +- 1 files changed, 1 insertions(+), 1 deletions(-) diff --git a/rc.sysinit b/rc.sysinit index 7b086fa..cb0144c 100755 --- a/rc.sysinit +++ b/rc.sysinit @@ -231,7 +231,7 @@ fi # Flush old locale settings and set user defined locale stat_busy "Setting Locale: ${LOCALE:=en_US}" echo "export LANG=$LOCALE" > /etc/profile.d/locale.sh && - chmod 0755 /etc/profile.d/locale.sh && stat_done || stat_fail +chmod 0755 /etc/profile.d/locale.sh && stat_done || stat_fail if [[ ${LOCALE,,} =~ utf ]]; then stat_busy "Setting Consoles to UTF-8 mode" -- 1.7.1

Prevent any word splitting where not intended. --- functions | 2 +- 1 files changed, 1 insertions(+), 1 deletions(-) diff --git a/functions b/functions index 71b49e3..ec043b6 100644 --- a/functions +++ b/functions @@ -417,7 +417,7 @@ fsck_reboot() { mount_all() { stat_busy "Mounting Local Filesystems" run_hook sysinit_premount - mount -a -t $NETFS -O no_netdev + mount -a -t "$NETFS" -O no_netdev run_hook sysinit_postmount stat_done } -- 1.7.1

Genaral scheme for hook positions is now: run_hook pre_foo if [[$WE_WANT_TO_DO_FOO]]]; then stat_busy "Doing foo" if [[$PRECONDITIONS_FOR_FOO_NOT_SATISFIED]]; then stat_fail else ... stat_done fi fi run hook post_foo rc.sysinit ----------- run_hook pre_foo [[$WE_WANT_TO_DO_FOO]] && status "Doing foo" foo run hook post_foo functions ------------ foo() { [[$PRECONDITIONS_FOR_FOO_NOT_SATISFIED]] && return 1 ... } Rationale Following this scheme as close as possible (without duplicating code and status messages) makes stuff more readable and uniform. Splitting kill_everything() into two new functions stop_all_daemons() and kill_all() also allows customization of either daemons stopping or process killing in an easy way. Suggested-by: Tom Gundersen --- functions | 36 +++++++++++------------------------- rc.shutdown | 8 +++++++- rc.single | 9 ++++++++- rc.sysinit | 24 +++++++++++++++++++++--- 4 files changed, 47 insertions(+), 30 deletions(-) diff --git a/functions b/functions index 7acb855..fd18694 100644 --- a/functions +++ b/functions @@ -265,10 +265,10 @@ add_omit_pids() { omit_pids+=( $@ ) } - -kill_everything() { - # $1 = where we are being called from. - # This is used to determine which hooks to run. +# Stop all daemons +# This function should *never* ever perform any other actions beside calling stop_daemon()! +# It might be used by a splash system etc. to get a list of daemons to be stopped. +stop_all_daemons() { # Find daemons NOT in the DAEMONS array. Shut these down first local daemon for daemon in /run/daemons/*; do @@ -283,10 +283,11 @@ kill_everything() { [[ ${DAEMONS[i]} = '!'* ]] && continue ck_daemon ${DAEMONS[i]#@} || stop_daemon ${DAEMONS[i]#@} done +} +kill_all() { # Terminate all processes stat_busy "Sending SIGTERM To Processes" - run_hook "$1_prekillall" killall5 -15 ${omit_pids[@]/#/-o } &>/dev/null sleep 5 stat_done @@ -295,8 +296,6 @@ kill_everything() { killall5 -9 ${omit_pids[@]/#/-o } &>/dev/null sleep 1 stat_done - - run_hook "$1_postkillall" } # Start/trigger UDev, load MODULES and settle UDev @@ -360,23 +359,14 @@ read_crypttab() { return $failed } +# Filesystem functions +# These can be overridden/reused for customizations like shutdown/loop-fsck. NETFS="nonfs,nonfs4,nosmbfs,nocifs,nocodafs,noncpfs,nosysfs,noshfs,nofuse,nofuseblk,noglusterfs,nodavfs" # Check local filesystems fsck_all() { - [[ -x $(type -P fsck) ]] || return 0 - stat_busy "Checking Filesystems" - FSCK_OUT=/dev/stdout - FSCK_ERR=/dev/stdout - FSCK_FD= - FORCEFSCK= - [[ -f /forcefsck ]] || in_array forcefsck $(< /proc/cmdline) && FORCEFSCK="-- -f" - run_hook sysinit_prefsck - fsck -A -T -C$FSCK_FD -a -t "$NETFS,noopts=_netdev" $FORCEFSCK >|$FSCK_OUT 2>|$FSCK_ERR - local -r fsckret=$? - (( fsckret <= 1 )) && stat_done || stat_fail - run_hook sysinit_postfsck - return $fsckret + fsck -A -T -C$FSCK_FD -a -t "$NETFS,noopts=_netdev" $FORCEFSCK >|$FSCK_OUT 2>|$FSCK_ERR + return $? } # Single-user login and/or automatic reboot after fsck (if needed) @@ -415,11 +405,7 @@ fsck_reboot() { } mount_all() { - stat_busy "Mounting Local Filesystems" - run_hook sysinit_premount - mount -a -t "$NETFS" -O no_netdev - run_hook sysinit_postmount - stat_done + mount -a -t "$NETFS" -O no_netdev } bootlogd_stop() { diff --git a/rc.shutdown b/rc.shutdown index fe42797..ed87eec 100755 --- a/rc.shutdown +++ b/rc.shutdown @@ -17,7 +17,13 @@ echo " " [[ -x /etc/rc.local.shutdown ]] && /etc/rc.local.shutdown -kill_everything shutdown +stop_all_daemons + +run_hook shutdown_prekillall + +kill_all + +run_hook shutdown_postkillall stat_busy "Saving Random Seed" RANDOM_SEED=/var/lib/misc/random-seed diff --git a/rc.single b/rc.single index d1efd41..21fe3be 100755 --- a/rc.single +++ b/rc.single @@ -11,7 +11,14 @@ export PATH="/usr/local/sbin:/usr/local/bin:/usr/sbin:/usr/bin:/sbin:/bin" run_hook single_start if [[ $PREVLEVEL != N ]]; then - kill_everything single + + stop_all_daemons + + run_hook single_prekillall + + kill_all + + run_hook single_postkillall # start up our mini logger until syslog takes over minilogd diff --git a/rc.sysinit b/rc.sysinit index cb0144c..c3610dc 100755 --- a/rc.sysinit +++ b/rc.sysinit @@ -174,9 +174,24 @@ if [[ -f /etc/crypttab && $CS ]] && grep -q ^[^#] /etc/crypttab; then fi # Check filesystems -fsck_all +FSCK_OUT=/dev/stdout +FSCK_ERR=/dev/stdout +FSCK_FD= +FORCEFSCK= +[[ -f /forcefsck ]] || is_in_array forcefsck $(< /proc/cmdline) && FORCEFSCK="-- -f" +run_hook sysinit_prefsck +if [[ -x $(type -P fsck) ]]; then + stat_busy "Checking Filesystems" + fsck_all + declare -r fsckret=$? + (( fsckret <= 1 )) && stat_done || stat_fail +else + declare -r fsckret=0 +fi +run_hook sysinit_postfsck + # Single-user login and/or automatic reboot if needed -fsck_reboot $? +fsck_reboot $fsckret status "Remounting Root Read/Write" \ mount -n -o remount,rw / @@ -193,7 +208,10 @@ if [[ ! -L /etc/mtab ]]; then fi # now mount all the local filesystems -mount_all +run_hook sysinit_premount +status "Mounting Local Filesystems" \ + mount_all +run_hook sysinit_postmount # enable monitoring of lvm2 groups, now that the filesystems are mounted rw [[ $USELVM = [Yy][Ee][Ss] && -x $(type -P lvm) && -d /sys/block ]] && -- 1.7.1

$FSCK_OUT/$FSCK_ERR might not be set when: * unset in some custom sysinit_prefsck hook * calling fsck() from a shutdown hook --- functions | 2 +- rc.sysinit | 6 +++--- 2 files changed, 4 insertions(+), 4 deletions(-) diff --git a/functions b/functions index fd18694..202db30 100644 --- a/functions +++ b/functions @@ -365,7 +365,7 @@ NETFS="nonfs,nonfs4,nosmbfs,nocifs,nocodafs,noncpfs,nosysfs,noshfs,nofuse,nofuse # Check local filesystems fsck_all() { - fsck -A -T -C$FSCK_FD -a -t "$NETFS,noopts=_netdev" $FORCEFSCK >|$FSCK_OUT 2>|$FSCK_ERR + fsck -A -T -C$FSCK_FD -a -t "$NETFS,noopts=_netdev" $FORCEFSCK return $? } diff --git a/rc.sysinit b/rc.sysinit index f64c9ad..5814007 100755 --- a/rc.sysinit +++ b/rc.sysinit @@ -174,8 +174,8 @@ if [[ -f /etc/crypttab && $CS ]] && grep -q ^[^#] /etc/crypttab; then fi # Check filesystems -FSCK_OUT=/dev/stdout -FSCK_ERR=/dev/stdout +FSCK_OUT= +FSCK_ERR= FSCK_FD= FORCEFSCK= [[ -f /forcefsck ]] || is_in_array forcefsck $(< /proc/cmdline) && FORCEFSCK="-- -f" @@ -183,7 +183,7 @@ declare -r FORCEFSCK run_hook sysinit_prefsck if [[ -x $(type -P fsck) ]]; then stat_busy "Checking Filesystems" - fsck_all + fsck_all >|${FSCK_OUT:-/dev/stdout} 2>|${FSCK_ERR:-/dev/stdout} declare -r fsckret=$? (( fsckret <= 1 )) && stat_done || stat_fail else -- 1.7.1

The exit code returned by the given command might be useful/expected, so don't discard but return it. --- functions | 10 ++++------ 1 files changed, 4 insertions(+), 6 deletions(-) diff --git a/functions b/functions index 202db30..6efcd52 100644 --- a/functions +++ b/functions @@ -156,12 +156,10 @@ stat_die() { status() { stat_busy "$1" shift - if "$@" &>/dev/null; then - stat_done - return 0 - fi - stat_fail - return 1 + "$@" &>/dev/null + local ret=$? + (( $ret == 0 )) && stat_done || stat_fail + return $ret } # usage : in_array( $needle, $haystack ) -- 1.7.1

killall5 returns 2 if it didn't find any processes to send to. Using this avoids sleeping longer than needed. This saves another up to six seconds of reboot/shutdown/go-single time. --- functions | 15 +++++++++++++-- 1 files changed, 13 insertions(+), 2 deletions(-) diff --git a/functions b/functions index caa9060..09655ab 100644 --- a/functions +++ b/functions @@ -290,14 +290,25 @@ stop_all_daemons() { kill_all() { # Terminate all processes + # and wait until killall5 reports all done or timeout + # Unfortunately killall5 does not support the 0 signal, so just + # use SIGCONT for checking (which should be ignored). stat_busy "Sending SIGTERM To Processes" + local i killall5 -15 ${omit_pids[@]/#/-o } &>/dev/null - sleep 5 + for (( i=0; i<20 && $?!=2; i++ )); do + sleep .25 # 1/4 second + killall5 -18 ${omit_pids[@]/#/-o } &>/dev/null + done stat_done stat_busy "Sending SIGKILL To Processes" + local i killall5 -9 ${omit_pids[@]/#/-o } &>/dev/null - sleep 1 + for (( i=0; i<4 && $?!=2; i++ )); do + sleep .25 # 1/4 second + killall5 -18 ${omit_pids[@]/#/-o } &>/dev/null + done stat_done } -- 1.7.1

We set $PATH in functions now, so use it. --- netfs | 8 ++++---- 1 files changed, 4 insertions(+), 4 deletions(-) diff --git a/netfs b/netfs index 8b42cd8..fff8482 100755 --- a/netfs +++ b/netfs @@ -9,9 +9,9 @@ rc=0 case "$1" in start) stat_busy "Mounting Network Filesystems" - /bin/mount -a -t nfs,nfs4,smbfs,codafs,ncpfs,cifs,shfs,glusterfs,fuse,fuseblk,fuse.glusterfs,davfs + mount -a -t nfs,nfs4,smbfs,codafs,ncpfs,cifs,shfs,glusterfs,fuse,fuseblk,fuse.glusterfs,davfs rc=$? - /bin/mount -a -O _netdev + mount -a -O _netdev if ((rc + $? > 0)); then stat_fail else @@ -21,9 +21,9 @@ case "$1" in ;; stop) stat_busy "Unmounting Network Filesystems" - /bin/umount -a -O _netdev + umount -a -O _netdev rc=$? - /bin/umount -a -t nfs,nfs4,smbfs,codafs,ncpfs,cifs,shfs,glusterfs,fuse,fuseblk,fuse.glusterfs,davfs + umount -a -t nfs,nfs4,smbfs,codafs,ncpfs,cifs,shfs,glusterfs,fuse,fuseblk,fuse.glusterfs,davfs if ((rc + $? > 0)); then stat_fail else -- 1.7.1

Currently $NETFS is used only for fsck and mount in rc.sysinit. Since we moved it into functions, we can use it in netfs too to get rid of redundancy. functions: * Move 'nosysfs' from $NETFS into the mount -a type list because thats obviously the only meaningfull place. * Add 'nofuse.glusterfs' to $NETFS to match the lists in netfs. * Remove all 'no'-prefixes from $NETFS to make it more useful and readable. * Add 'no'-prefixes to fsck and mount -a commands by parameter substitution. netfs: * Instead of the literals use $NETFS from functions which contains the same types now --- functions | 6 +++--- netfs | 4 ++-- 2 files changed, 5 insertions(+), 5 deletions(-) diff --git a/functions b/functions index 09655ab..ae60771 100644 --- a/functions +++ b/functions @@ -375,11 +375,11 @@ read_crypttab() { # Filesystem functions # These can be overridden/reused for customizations like shutdown/loop-fsck. -NETFS="nonfs,nonfs4,nosmbfs,nocifs,nocodafs,noncpfs,nosysfs,noshfs,nofuse,nofuseblk,noglusterfs,nodavfs" +NETFS="nfs,nfs4,smbfs,cifs,codafs,ncpfs,shfs,fuse,fuseblk,glusterfs,davfs,fuse.glusterfs" # Check local filesystems fsck_all() { - fsck -A -T -C$FSCK_FD -a -t "$NETFS,noopts=_netdev" $FORCEFSCK + fsck -A -T -C$FSCK_FD -a -t "no${NETFS//,/,no},noopts=_netdev" $FORCEFSCK return $? } @@ -419,7 +419,7 @@ fsck_reboot() { } mount_all() { - mount -a -t "$NETFS" -O no_netdev + mount -a -t "nosysfs,no${NETFS//,/,no}" -O no_netdev } bootlogd_stop() { diff --git a/netfs b/netfs index ffda681..ea7e4eb 100755 --- a/netfs +++ b/netfs @@ -7,7 +7,7 @@ case "$1" in start) stat_busy "Mounting Network Filesystems" - mount -a -t nfs,nfs4,smbfs,codafs,ncpfs,cifs,shfs,glusterfs,fuse,fuseblk,fuse.glusterfs,davfs + mount -a -t "$NETFS" rc=$? mount -a -O _netdev (( rc || $? )) && stat_die @@ -18,7 +18,7 @@ case "$1" in stat_busy "Unmounting Network Filesystems" umount -a -O _netdev rc=$? - umount -a -t nfs,nfs4,smbfs,codafs,ncpfs,cifs,shfs,glusterfs,fuse,fuseblk,fuse.glusterfs,davfs + umount -a -t "$NETFS" (( rc || $? )) && stat_die rm_daemon netfs stat_done -- 1.7.1

Instead of guessing about symlinks, use stat to check whether the filesystem refered to is actually a tmpfs. Then always clean up if it isn't. --- rc.sysinit | 4 ++-- 1 files changed, 2 insertions(+), 2 deletions(-) diff --git a/rc.sysinit b/rc.sysinit index 7357582..1dd3d9a 100755 --- a/rc.sysinit +++ b/rc.sysinit @@ -228,8 +228,8 @@ RANDOM_SEED=/var/lib/misc/random-seed stat_busy "Removing Leftover Files" rm -rf /etc/{nologin,shutdownpid} /forcefsck /tmp/* /tmp/.* /var/run/daemons &>/dev/null - [[ ! -L /var/lock ]] && rm -rf /var/lock/* - if [[ ! -L /var/run && -d /var/run ]]; then + [[ -d /var/lock && $(stat -f -c %T -L /var/lock) != tmpfs ]] && rm -rf /var/lock/* + if [[ -d /var/run && $(stat -f -c %T -L /var/run) != tmpfs ]]; then find /var/run/ \! -type d -delete ln -s /run/daemons /var/run/daemons fi -- 1.7.1

Rationale: Instead of ignoring any errors of all commands but the last within a status block it is much better to use trap to catch them and report 'FAIL'. One might for example miss the utmp group which is needed to propperly install /var/run/utmp. This also makes the code a bit more simple and readable. Note: We enable this explicitely (by calling stat_err_trap) because most daemon scriptlets don't work well with this by now. Moreover we don't use 'set -E' to avoid breaking/complicating [custom/hook] functions. --- functions | 10 ++++++++++ netfs | 12 ++++-------- rc.shutdown | 2 ++ rc.single | 2 ++ rc.sysinit | 56 +++++++++++++++++++++++++++++++------------------------- 5 files changed, 49 insertions(+), 33 deletions(-) diff --git a/functions b/functions index ae60771..58f7fa4 100644 --- a/functions +++ b/functions @@ -125,11 +125,17 @@ stat_bkgd() { printf " ${C_OTHER}[${C_BKGD}BKGD${C_OTHER}]${C_CLEAR} " } +# Allow to catch any errors and get the exit-code of the last failing command +stat_err_trap() { + trap 'STAT_ERR=$?' ERR +} + stat_busy() { printf "${C_OTHER}${PREFIX_REG} ${C_MAIN}${1}${C_CLEAR} " printf "${SAVE_POSITION}" deltext printf " ${C_OTHER}[${C_BUSY}BUSY${C_OTHER}]${C_CLEAR} " + STAT_ERR=0 } stat_append() { @@ -139,6 +145,10 @@ stat_append() { } stat_done() { + if (( STAT_ERR )); then + stat_fail + return $STAT_ERR + fi deltext printf " ${C_OTHER}[${C_DONE}DONE${C_OTHER}]${C_CLEAR} \n" } diff --git a/netfs b/netfs index ea7e4eb..4dc91ef 100755 --- a/netfs +++ b/netfs @@ -4,24 +4,20 @@ . /etc/rc.conf . /etc/rc.d/functions +stat_err_trap + case "$1" in start) stat_busy "Mounting Network Filesystems" mount -a -t "$NETFS" - rc=$? mount -a -O _netdev - (( rc || $? )) && stat_die - add_daemon netfs - stat_done + stat_done && add_daemon netfs || exit 1 ;; stop) stat_busy "Unmounting Network Filesystems" umount -a -O _netdev - rc=$? umount -a -t "$NETFS" - (( rc || $? )) && stat_die - rm_daemon netfs - stat_done + stat_done && rm_daemon netfs || exit 1 ;; restart) $0 stop diff --git a/rc.shutdown b/rc.shutdown index ed87eec..db4f12b 100755 --- a/rc.shutdown +++ b/rc.shutdown @@ -6,6 +6,8 @@ . /etc/rc.conf . /etc/rc.d/functions +stat_err_trap + run_hook shutdown_start # avoid staircase effect diff --git a/rc.single b/rc.single index 7a87c72..b5c6e66 100755 --- a/rc.single +++ b/rc.single @@ -6,6 +6,8 @@ . /etc/rc.conf . /etc/rc.d/functions +stat_err_trap + run_hook single_start if [[ $PREVLEVEL != N ]]; then diff --git a/rc.sysinit b/rc.sysinit index 1dd3d9a..13ca0be 100755 --- a/rc.sysinit +++ b/rc.sysinit @@ -6,6 +6,8 @@ . /etc/rc.conf . /etc/rc.d/functions +stat_err_trap + echo " " printhl "Arch Linux\n" printhl "${C_H2}http://www.archlinux.org" @@ -14,17 +16,19 @@ printsep run_hook sysinit_start # mount /proc, /sys, /run, /dev, /run/lock, /dev/pts, /dev/shm (the api filesystems) -mountpoint -q /proc || mount -n -t proc proc /proc -o nosuid,noexec,nodev -mountpoint -q /sys || mount -n -t sysfs sys /sys -o nosuid,noexec,nodev -mountpoint -q /run || mount -n -t tmpfs run /run -o mode=0755,size=10M,nosuid,nodev -mountpoint -q /dev || mount -n -t devtmpfs udev /dev -o mode=0755,size=10M,nosuid &>/dev/null \ - || mount -n -t tmpfs udev /dev -o mode=0755,size=10M,nosuid -mkdir -p -m 1777 /run/lock -mkdir -p /dev/{pts,shm} -mountpoint -q /dev/pts || mount -n /dev/pts &>/dev/null \ - || mount -n -t devpts devpts /dev/pts -o mode=0620,gid=5,nosuid,noexec -mountpoint -q /dev/shm || mount -n /dev/shm &>/dev/null \ - || mount -n -t tmpfs shm /dev/shm -o mode=1777,nosuid,nodev +stat_busy "Mounting API Filesystems" + mountpoint -q /proc || mount -n -t proc proc /proc -o nosuid,noexec,nodev + mountpoint -q /sys || mount -n -t sysfs sys /sys -o nosuid,noexec,nodev + mountpoint -q /run || mount -n -t tmpfs run /run -o mode=0755,size=10M,nosuid,nodev + mountpoint -q /dev || mount -n -t devtmpfs udev /dev -o mode=0755,size=10M,nosuid &>/dev/null \ + || mount -n -t tmpfs udev /dev -o mode=0755,size=10M,nosuid + mkdir -p -m 1777 /run/lock + mkdir -p /dev/{pts,shm} + mountpoint -q /dev/pts || mount -n /dev/pts &>/dev/null \ + || mount -n -t devpts devpts /dev/pts -o mode=0620,gid=5,nosuid,noexec + mountpoint -q /dev/shm || mount -n /dev/shm &>/dev/null \ + || mount -n -t tmpfs shm /dev/shm -o mode=1777,nosuid,nodev +stat_done # remount root ro to allow for fsck later on, we remount now to # make sure nothing can open files rw on root which would block a remount @@ -46,7 +50,7 @@ esac if [[ $HWCLOCK_PARAMS ]]; then stat_busy "Adjusting system time and setting kernel timezone" # enable rtc access - modprobe -q -a rtc-cmos rtc genrtc + modprobe -q -a rtc-cmos rtc genrtc || : # If devtmpfs is used, the required RTC device already exists now # Otherwise, create whatever device is available if ! [[ -c /dev/rtc || -c /dev/rtc0 ]]; then @@ -65,7 +69,8 @@ if [[ $HWCLOCK_PARAMS ]]; then # is used. If HARDWARECLOCK is not set in rc.conf, the value in # /var/lib/hwclock/adjfile is used (in this case /var can not be a separate # partition). - TZ=$TIMEZONE hwclock $HWCLOCK_PARAMS && stat_done || stat_fail + TZ=$TIMEZONE hwclock $HWCLOCK_PARAMS + stat_done fi # Start/trigger UDev, load MODULES and settle UDev @@ -89,7 +94,7 @@ activate_vgs # Set up non-root encrypted partition mappings if [[ -f /etc/crypttab && $CS ]] && grep -q ^[^#] /etc/crypttab; then stat_busy "Unlocking encrypted volumes:" - modprobe -q dm-crypt 2>/dev/null + modprobe -q dm-crypt 2>/dev/null || : do_unlock() { # $1 = requested name # $2 = source device @@ -167,8 +172,9 @@ if [[ -f /etc/crypttab && $CS ]] && grep -q ^[^#] /etc/crypttab; then fi return $failed } - crypto_unlocked=0 - read_crypttab do_unlock && stat_done || stat_fail + crypto_unlocked=0 + read_crypttab do_unlock + stat_done # Maybe someone has LVM on an encrypted block device (( crypto_unlocked == 1 )) && activate_vgs fi @@ -177,14 +183,13 @@ fi [[ -f /forcefsck ]] || is_in_array forcefsck $(< /proc/cmdline) && FORCEFSCK="-- -f" declare -r FORCEFSCK run_hook sysinit_prefsck +fsckret=0 if [[ -x $(type -P fsck) ]]; then stat_busy "Checking Filesystems" - fsck_all >|${FSCK_OUT:-/dev/stdout} 2>|${FSCK_ERR:-/dev/stdout} - declare -r fsckret=$? + fsck_all >|${FSCK_OUT:-/dev/stdout} 2>|${FSCK_ERR:-/dev/stdout} || fsckret=$? (( fsckret <= 1 )) && stat_done || stat_fail -else - declare -r fsckret=0 fi +declare -r fsckret run_hook sysinit_postfsck # Single-user login and/or automatic reboot if needed @@ -201,7 +206,7 @@ if [[ ! -L /etc/mtab ]]; then else cat /proc/mounts >| /etc/mtab fi - (( $? == 0 )) && stat_done || stat_fail + stat_done fi # now mount all the local filesystems @@ -227,7 +232,7 @@ RANDOM_SEED=/var/lib/misc/random-seed cp $RANDOM_SEED /dev/urandom stat_busy "Removing Leftover Files" - rm -rf /etc/{nologin,shutdownpid} /forcefsck /tmp/* /tmp/.* /var/run/daemons &>/dev/null + rm -rf /etc/{nologin,shutdownpid} /forcefsck /tmp/* /tmp/.[^.]* /tmp/..?* /var/run/daemons [[ -d /var/lock && $(stat -f -c %T -L /var/lock) != tmpfs ]] && rm -rf /var/lock/* if [[ -d /var/run && $(stat -f -c %T -L /var/run) != tmpfs ]]; then find /var/run/ \! -type d -delete @@ -245,8 +250,9 @@ fi # Flush old locale settings and set user defined locale stat_busy "Setting Locale: ${LOCALE:=en_US}" - echo "export LANG=$LOCALE" > /etc/profile.d/locale.sh && -chmod 0755 /etc/profile.d/locale.sh && stat_done || stat_fail + echo "export LANG=$LOCALE" > /etc/profile.d/locale.sh + chmod 0755 /etc/profile.d/locale.sh +stat_done if [[ ${LOCALE,,} =~ utf ]]; then stat_busy "Setting Consoles to UTF-8 mode" @@ -282,7 +288,7 @@ stat_busy "Saving dmesg Log" else install -Tm 0644 <( dmesg ) /var/log/dmesg.log fi -(( $? == 0 )) && stat_done || stat_fail +stat_done run_hook sysinit_end -- 1.7.1

Rationale: Instead of ignoring any errors of all commands but the last within a status block it is much better to use trap to catch them and report 'FAIL'. One might for example miss the utmp group which is needed to propperly install /var/run/utmp. This also makes the code a bit more simple and readable. Note: We enable this explicitely (by calling stat_err_trap) because most daemon scriptlets don't work well with this by now. Moreover we don't use 'set -E' to avoid breaking/complicating [custom/hook] functions. --- functions | 10 ++++++++++ netfs | 12 ++++-------- rc.shutdown | 2 ++ rc.single | 2 ++ rc.sysinit | 59 +++++++++++++++++++++++++++++++++-------------------------- 5 files changed, 51 insertions(+), 34 deletions(-) diff --git a/functions b/functions index d1d2947..ac5394c 100644 --- a/functions +++ b/functions @@ -125,11 +125,17 @@ stat_bkgd() { printf " ${C_OTHER}[${C_BKGD}BKGD${C_OTHER}]${C_CLEAR} " } +# Allow to catch any errors and get the exit-code of the last failing command +stat_err_trap() { + trap 'STAT_ERR=$?' ERR +} + stat_busy() { printf "${C_OTHER}${PREFIX_REG} ${C_MAIN}${1}${C_CLEAR} " printf "${SAVE_POSITION}" deltext printf " ${C_OTHER}[${C_BUSY}BUSY${C_OTHER}]${C_CLEAR} " + STAT_ERR=0 } stat_append() { @@ -139,6 +145,10 @@ stat_append() { } stat_done() { + if (( STAT_ERR )); then + stat_fail + return $STAT_ERR + fi deltext printf " ${C_OTHER}[${C_DONE}DONE${C_OTHER}]${C_CLEAR} \n" } diff --git a/netfs b/netfs index ea7e4eb..4dc91ef 100755 --- a/netfs +++ b/netfs @@ -4,24 +4,20 @@ . /etc/rc.conf . /etc/rc.d/functions +stat_err_trap + case "$1" in start) stat_busy "Mounting Network Filesystems" mount -a -t "$NETFS" - rc=$? mount -a -O _netdev - (( rc || $? )) && stat_die - add_daemon netfs - stat_done + stat_done && add_daemon netfs || exit 1 ;; stop) stat_busy "Unmounting Network Filesystems" umount -a -O _netdev - rc=$? umount -a -t "$NETFS" - (( rc || $? )) && stat_die - rm_daemon netfs - stat_done + stat_done && rm_daemon netfs || exit 1 ;; restart) $0 stop diff --git a/rc.shutdown b/rc.shutdown index ed87eec..db4f12b 100755 --- a/rc.shutdown +++ b/rc.shutdown @@ -6,6 +6,8 @@ . /etc/rc.conf . /etc/rc.d/functions +stat_err_trap + run_hook shutdown_start # avoid staircase effect diff --git a/rc.single b/rc.single index 7a87c72..b5c6e66 100755 --- a/rc.single +++ b/rc.single @@ -6,6 +6,8 @@ . /etc/rc.conf . /etc/rc.d/functions +stat_err_trap + run_hook single_start if [[ $PREVLEVEL != N ]]; then diff --git a/rc.sysinit b/rc.sysinit index 97c16c8..be8eb05 100755 --- a/rc.sysinit +++ b/rc.sysinit @@ -6,6 +6,8 @@ . /etc/rc.conf . /etc/rc.d/functions +stat_err_trap + echo " " printhl "Arch Linux\n" printhl "${C_H2}http://www.archlinux.org" @@ -14,17 +16,19 @@ printsep run_hook sysinit_start # mount /proc, /sys, /run, /dev, /run/lock, /dev/pts, /dev/shm (the api filesystems) -mountpoint -q /proc || mount -n -t proc proc /proc -o nosuid,noexec,nodev -mountpoint -q /sys || mount -n -t sysfs sys /sys -o nosuid,noexec,nodev -mountpoint -q /run || mount -n -t tmpfs run /run -o mode=0755,size=10M,nosuid,nodev -mountpoint -q /dev || mount -n -t devtmpfs udev /dev -o mode=0755,size=10M,nosuid &>/dev/null \ - || mount -n -t tmpfs udev /dev -o mode=0755,size=10M,nosuid -mkdir -p -m 1777 /run/lock -mkdir -p /dev/{pts,shm} -mountpoint -q /dev/pts || mount -n /dev/pts &>/dev/null \ - || mount -n -t devpts devpts /dev/pts -o mode=0620,gid=5,nosuid,noexec -mountpoint -q /dev/shm || mount -n /dev/shm &>/dev/null \ - || mount -n -t tmpfs shm /dev/shm -o mode=1777,nosuid,nodev +stat_busy "Mounting API Filesystems" + mountpoint -q /proc || mount -n -t proc proc /proc -o nosuid,noexec,nodev + mountpoint -q /sys || mount -n -t sysfs sys /sys -o nosuid,noexec,nodev + mountpoint -q /run || mount -n -t tmpfs run /run -o mode=0755,size=10M,nosuid,nodev + mountpoint -q /dev || mount -n -t devtmpfs udev /dev -o mode=0755,size=10M,nosuid &>/dev/null \ + || mount -n -t tmpfs udev /dev -o mode=0755,size=10M,nosuid + mkdir -p -m 1777 /run/lock + mkdir -p /dev/{pts,shm} + mountpoint -q /dev/pts || mount -n /dev/pts &>/dev/null \ + || mount -n -t devpts devpts /dev/pts -o mode=0620,gid=5,nosuid,noexec + mountpoint -q /dev/shm || mount -n /dev/shm &>/dev/null \ + || mount -n -t tmpfs shm /dev/shm -o mode=1777,nosuid,nodev +stat_done # remount root ro to allow for fsck later on, we remount now to # make sure nothing can open files rw on root which would block a remount @@ -46,7 +50,7 @@ esac if [[ $HWCLOCK_PARAMS ]]; then stat_busy "Adjusting system time and setting kernel timezone" # enable rtc access - modprobe -q -a rtc-cmos rtc genrtc + modprobe -q -a rtc-cmos rtc genrtc || : # If devtmpfs is used, the required RTC device already exists now # Otherwise, create whatever device is available if ! [[ -c /dev/rtc || -c /dev/rtc0 ]]; then @@ -65,7 +69,8 @@ if [[ $HWCLOCK_PARAMS ]]; then # is used. If HARDWARECLOCK is not set in rc.conf, the value in # /var/lib/hwclock/adjfile is used (in this case /var can not be a separate # partition). - TZ=$TIMEZONE hwclock $HWCLOCK_PARAMS && stat_done || stat_fail + TZ=$TIMEZONE hwclock $HWCLOCK_PARAMS + stat_done fi # Start/trigger UDev, load MODULES and settle UDev @@ -89,7 +94,7 @@ activate_vgs # Set up non-root encrypted partition mappings if [[ -f /etc/crypttab && $CS ]] && grep -q ^[^#] /etc/crypttab; then stat_busy "Unlocking encrypted volumes:" - modprobe -q dm-crypt 2>/dev/null + modprobe -q dm-crypt 2>/dev/null || : do_unlock() { # $1 = requested name # $2 = source device @@ -167,8 +172,9 @@ if [[ -f /etc/crypttab && $CS ]] && grep -q ^[^#] /etc/crypttab; then fi return $failed } - crypto_unlocked=0 - read_crypttab do_unlock && stat_done || stat_fail + crypto_unlocked=0 + read_crypttab do_unlock + stat_done # Maybe someone has LVM on an encrypted block device (( crypto_unlocked == 1 )) && activate_vgs fi @@ -177,14 +183,13 @@ fi [[ -f /forcefsck || " $(< /proc/cmdline) " =~ [[:blank:]]forcefsck[[:blank:]] ]] && FORCEFSCK="-- -f" declare -r FORCEFSCK run_hook sysinit_prefsck +fsckret=0 if [[ -x $(type -P fsck) ]]; then stat_busy "Checking Filesystems" - fsck_all >|${FSCK_OUT:-/dev/stdout} 2>|${FSCK_ERR:-/dev/stdout} - declare -r fsckret=$? + fsck_all >|${FSCK_OUT:-/dev/stdout} 2>|${FSCK_ERR:-/dev/stdout} || fsckret=$? (( fsckret <= 1 )) && stat_done || stat_fail -else - declare -r fsckret=0 fi +declare -r fsckret run_hook sysinit_postfsck # Single-user login and/or automatic reboot if needed @@ -201,7 +206,7 @@ if [[ ! -L /etc/mtab ]]; then else cat /proc/mounts >| /etc/mtab fi - (( $? == 0 )) && stat_done || stat_fail + stat_done fi # now mount all the local filesystems @@ -227,7 +232,7 @@ RANDOM_SEED=/var/lib/misc/random-seed cp $RANDOM_SEED /dev/urandom stat_busy "Removing Leftover Files" - rm -rf /etc/{nologin,shutdownpid} /forcefsck /tmp/* /tmp/.* /var/run/daemons &>/dev/null + rm -rf /etc/{nologin,shutdownpid} /forcefsck /tmp/* /tmp/.[^.]* /tmp/..?* /var/run/daemons [[ -d /var/lock && $(stat -f -c %T -L /var/lock) != tmpfs ]] && rm -rf /var/lock/* if [[ -d /var/run && $(stat -f -c %T -L /var/run) != tmpfs ]]; then find /var/run/ \! -type d -delete @@ -240,13 +245,15 @@ stat_done if [[ $HOSTNAME ]]; then stat_busy "Setting Hostname: $HOSTNAME" - echo "$HOSTNAME" >| /proc/sys/kernel/hostname && stat_done || stat_fail + echo "$HOSTNAME" >| /proc/sys/kernel/hostname + stat_done fi # Flush old locale settings and set user defined locale stat_busy "Setting Locale: ${LOCALE:=en_US}" - echo "export LANG=$LOCALE" > /etc/profile.d/locale.sh && -chmod 0755 /etc/profile.d/locale.sh && stat_done || stat_fail + echo "export LANG=$LOCALE" > /etc/profile.d/locale.sh + chmod 0755 /etc/profile.d/locale.sh +stat_done if [[ ${LOCALE,,} =~ utf ]]; then stat_busy "Setting Consoles to UTF-8 mode" @@ -282,7 +289,7 @@ stat_busy "Saving dmesg Log" else install -Tm 0644 <( dmesg ) /var/log/dmesg.log fi -(( $? == 0 )) && stat_done || stat_fail +stat_done run_hook sysinit_end -- 1.7.1

This is just a bit simpler. in_array() should be fixed to be non daemon specific in a later patch. --- functions | 2 +- 1 files changed, 1 insertions(+), 1 deletions(-) diff --git a/functions b/functions index 5bfeb77..f564f3d 100644 --- a/functions +++ b/functions @@ -272,7 +272,7 @@ stop_all_daemons() { for daemon in /run/daemons/*; do [[ -f $daemon ]] || continue daemon=${daemon##*/} - in_array "$daemon" "${DAEMONS[@]}" || stop_daemon "$daemon" + ck_autostart "$daemon" && stop_daemon "$daemon" done # Shutdown daemons in reverse order -- 1.7.1

For checking whether a daemon is started via the DAEMONS array we use ck_autostart() now. So make in_array() generic as the name suggests instead of removing '@'-prefixes from array members behind ones back. Suggested-by: Dave Reisner --- functions | 2 +- 1 files changed, 1 insertions(+), 1 deletions(-) diff --git a/functions b/functions index f564f3d..000ee0e 100644 --- a/functions +++ b/functions @@ -169,7 +169,7 @@ in_array() { local needle=$1; shift local item for item; do - [[ ${item#@} = $needle ]] && return 0 + [[ $item = $needle ]] && return 0 done return 1 # Not Found } -- 1.7.1

Whitespace changes are introduced because of out status block indentation rule. I always attempted to make things more readable and elegant. First i changed five line if/stat_{done,fail} clauses into one liners using '&&' and '||'. Now i change these into two lines without any conditional constructs because whether stat_done or stat_fail is actually done no longer depends on the last command only, but on all commands within the status block. I thing the code would look better then ever before with this now. As said in the commit, it allows to catch errors caused by missing utmp group. It would also allow to catch errors caused by missing API-FS mountpoints or missing binaries called somewhere in the middle of a status block etc. I think we should avoid to say '[DONE]' in cases where actually only the last action succeeded in a multi actions block like 'Removing Leftover Files'. To get more fine grained error messages, we would just need to remove all std_err redirections where ever possible as done for cleaning /tmp/ here (by using correct patterns). In fact i think this makes the code more readable and avoiding hidden errors would make troubleshooting (a system where initscripts is installed) much easier. Actually all kinds of subshells and also function bodies aren't involved here as long as we don't use 'set -E'. trap 'echo err' ERR cat <( false ) # <<< you see nothing here unless you have done 'set -E' Yes, i also like TOFU (sometimes). -- Kurt Dave Reisner, 2011-07-10 21:30:

You've got large quantities of non-functional changes in here (whitespace churn), mixed in with reverts to your previous attempts at turning everything into a single line. Can we pick a direction and stick with it? This all seems like a lot of cleverness to accomplish... well... I'm not sure. What exactly _does_ this accomplish that we didn't previously have? If we want to be more fine grained about error checking as your rationale advertises, then let's do that so we can report a more detailed error than "[FAIL]".

I'm sure I've mentioned this before, but we value simplicity and readability in the initscripts. This is crucial code that cannot go foul because of some bash side effect we haven't considered. I'd rather be a little more verbose and make this easier to troubleshoot than to have to worry about write errors on process substitutions triggering magical ERR trap causing an echo somewhere to claim FAIL.

Yes, I know I'm top posting.

d

On Sun, Jul 10, 2011 at 08:58:39PM +0200, Kurt J. Bosch wrote:

...

Rationale: Instead of ignoring any errors of all commands but the last within a status block it is much better to use trap to catch them and report 'FAIL'. One might for example miss the utmp group which is needed to propperly install /var/run/utmp. This also makes the code a bit more simple and readable.

Note: We enable this explicitely (by calling stat_err_trap) because most daemon scriptlets don't work well with this by now. Moreover we don't use 'set -E' to avoid breaking/complicating [custom/hook] functions. --- functions | 10 ++++++++++ netfs | 12 ++++-------- rc.shutdown | 2 ++ rc.single | 2 ++ rc.sysinit | 59 +++++++++++++++++++++++++++++++++-------------------------- 5 files changed, 51 insertions(+), 34 deletions(-)

diff --git a/functions b/functions index d1d2947..ac5394c 100644 --- a/functions +++ b/functions @@ -125,11 +125,17 @@ stat_bkgd() { printf " ${C_OTHER}[${C_BKGD}BKGD${C_OTHER}]${C_CLEAR} " }

+# Allow to catch any errors and get the exit-code of the last failing command +stat_err_trap() { + trap 'STAT_ERR=$?' ERR +} + stat_busy() { printf "${C_OTHER}${PREFIX_REG} ${C_MAIN}${1}${C_CLEAR} " printf "${SAVE_POSITION}" deltext printf " ${C_OTHER}[${C_BUSY}BUSY${C_OTHER}]${C_CLEAR} " + STAT_ERR=0 }

stat_append() { @@ -139,6 +145,10 @@ stat_append() { }

stat_done() { + if (( STAT_ERR )); then + stat_fail + return $STAT_ERR + fi deltext printf " ${C_OTHER}[${C_DONE}DONE${C_OTHER}]${C_CLEAR} \n" } diff --git a/netfs b/netfs index ea7e4eb..4dc91ef 100755 --- a/netfs +++ b/netfs @@ -4,24 +4,20 @@ . /etc/rc.conf . /etc/rc.d/functions

+stat_err_trap + case "$1" in start) stat_busy "Mounting Network Filesystems" mount -a -t "$NETFS" - rc=$? mount -a -O _netdev - (( rc || $? ))&& stat_die - add_daemon netfs - stat_done + stat_done&& add_daemon netfs || exit 1 ;; stop) stat_busy "Unmounting Network Filesystems" umount -a -O _netdev - rc=$? umount -a -t "$NETFS" - (( rc || $? ))&& stat_die - rm_daemon netfs - stat_done + stat_done&& rm_daemon netfs || exit 1 ;; restart) $0 stop diff --git a/rc.shutdown b/rc.shutdown index ed87eec..db4f12b 100755 --- a/rc.shutdown +++ b/rc.shutdown @@ -6,6 +6,8 @@ . /etc/rc.conf . /etc/rc.d/functions

+stat_err_trap + run_hook shutdown_start

# avoid staircase effect diff --git a/rc.single b/rc.single index 7a87c72..b5c6e66 100755 --- a/rc.single +++ b/rc.single @@ -6,6 +6,8 @@ . /etc/rc.conf . /etc/rc.d/functions

+stat_err_trap + run_hook single_start

if [[ $PREVLEVEL != N ]]; then diff --git a/rc.sysinit b/rc.sysinit index 97c16c8..be8eb05 100755 --- a/rc.sysinit +++ b/rc.sysinit @@ -6,6 +6,8 @@ . /etc/rc.conf . /etc/rc.d/functions

+stat_err_trap + echo " " printhl "Arch Linux\n" printhl "${C_H2}http://www.archlinux.org" @@ -14,17 +16,19 @@ printsep run_hook sysinit_start

# mount /proc, /sys, /run, /dev, /run/lock, /dev/pts, /dev/shm (the api filesystems) -mountpoint -q /proc || mount -n -t proc proc /proc -o nosuid,noexec,nodev -mountpoint -q /sys || mount -n -t sysfs sys /sys -o nosuid,noexec,nodev -mountpoint -q /run || mount -n -t tmpfs run /run -o mode=0755,size=10M,nosuid,nodev -mountpoint -q /dev || mount -n -t devtmpfs udev /dev -o mode=0755,size=10M,nosuid&>/dev/null \ - || mount -n -t tmpfs udev /dev -o mode=0755,size=10M,nosuid -mkdir -p -m 1777 /run/lock -mkdir -p /dev/{pts,shm} -mountpoint -q /dev/pts || mount -n /dev/pts&>/dev/null \ - || mount -n -t devpts devpts /dev/pts -o mode=0620,gid=5,nosuid,noexec -mountpoint -q /dev/shm || mount -n /dev/shm&>/dev/null \ - || mount -n -t tmpfs shm /dev/shm -o mode=1777,nosuid,nodev +stat_busy "Mounting API Filesystems" + mountpoint -q /proc || mount -n -t proc proc /proc -o nosuid,noexec,nodev + mountpoint -q /sys || mount -n -t sysfs sys /sys -o nosuid,noexec,nodev + mountpoint -q /run || mount -n -t tmpfs run /run -o mode=0755,size=10M,nosuid,nodev + mountpoint -q /dev || mount -n -t devtmpfs udev /dev -o mode=0755,size=10M,nosuid&>/dev/null \ + || mount -n -t tmpfs udev /dev -o mode=0755,size=10M,nosuid + mkdir -p -m 1777 /run/lock + mkdir -p /dev/{pts,shm} + mountpoint -q /dev/pts || mount -n /dev/pts&>/dev/null \ + || mount -n -t devpts devpts /dev/pts -o mode=0620,gid=5,nosuid,noexec + mountpoint -q /dev/shm || mount -n /dev/shm&>/dev/null \ + || mount -n -t tmpfs shm /dev/shm -o mode=1777,nosuid,nodev +stat_done

# remount root ro to allow for fsck later on, we remount now to # make sure nothing can open files rw on root which would block a remount @@ -46,7 +50,7 @@ esac if [[ $HWCLOCK_PARAMS ]]; then stat_busy "Adjusting system time and setting kernel timezone" # enable rtc access - modprobe -q -a rtc-cmos rtc genrtc + modprobe -q -a rtc-cmos rtc genrtc || : # If devtmpfs is used, the required RTC device already exists now # Otherwise, create whatever device is available if ! [[ -c /dev/rtc || -c /dev/rtc0 ]]; then @@ -65,7 +69,8 @@ if [[ $HWCLOCK_PARAMS ]]; then # is used. If HARDWARECLOCK is not set in rc.conf, the value in # /var/lib/hwclock/adjfile is used (in this case /var can not be a separate # partition). - TZ=$TIMEZONE hwclock $HWCLOCK_PARAMS&& stat_done || stat_fail + TZ=$TIMEZONE hwclock $HWCLOCK_PARAMS + stat_done fi

# Start/trigger UDev, load MODULES and settle UDev @@ -89,7 +94,7 @@ activate_vgs # Set up non-root encrypted partition mappings if [[ -f /etc/crypttab&& $CS ]]&& grep -q ^[^#] /etc/crypttab; then stat_busy "Unlocking encrypted volumes:" - modprobe -q dm-crypt 2>/dev/null + modprobe -q dm-crypt 2>/dev/null || : do_unlock() { # $1 = requested name # $2 = source device @@ -167,8 +172,9 @@ if [[ -f /etc/crypttab&& $CS ]]&& grep -q ^[^#] /etc/crypttab; then fi return $failed } - crypto_unlocked=0 - read_crypttab do_unlock&& stat_done || stat_fail + crypto_unlocked=0 + read_crypttab do_unlock + stat_done # Maybe someone has LVM on an encrypted block device (( crypto_unlocked == 1 ))&& activate_vgs fi @@ -177,14 +183,13 @@ fi [[ -f /forcefsck || " $(< /proc/cmdline) " =~ [[:blank:]]forcefsck[[:blank:]] ]]&& FORCEFSCK="-- -f" declare -r FORCEFSCK run_hook sysinit_prefsck +fsckret=0 if [[ -x $(type -P fsck) ]]; then stat_busy "Checking Filesystems" - fsck_all>|${FSCK_OUT:-/dev/stdout} 2>|${FSCK_ERR:-/dev/stdout} - declare -r fsckret=$? + fsck_all>|${FSCK_OUT:-/dev/stdout} 2>|${FSCK_ERR:-/dev/stdout} || fsckret=$? (( fsckret<= 1 ))&& stat_done || stat_fail -else - declare -r fsckret=0 fi +declare -r fsckret run_hook sysinit_postfsck

# Single-user login and/or automatic reboot if needed @@ -201,7 +206,7 @@ if [[ ! -L /etc/mtab ]]; then else cat /proc/mounts>| /etc/mtab fi - (( $? == 0 ))&& stat_done || stat_fail + stat_done fi

# now mount all the local filesystems @@ -227,7 +232,7 @@ RANDOM_SEED=/var/lib/misc/random-seed cp $RANDOM_SEED /dev/urandom

stat_busy "Removing Leftover Files" - rm -rf /etc/{nologin,shutdownpid} /forcefsck /tmp/* /tmp/.* /var/run/daemons&>/dev/null + rm -rf /etc/{nologin,shutdownpid} /forcefsck /tmp/* /tmp/.[^.]* /tmp/..?* /var/run/daemons [[ -d /var/lock&& $(stat -f -c %T -L /var/lock) != tmpfs ]]&& rm -rf /var/lock/* if [[ -d /var/run&& $(stat -f -c %T -L /var/run) != tmpfs ]]; then find /var/run/ \! -type d -delete @@ -240,13 +245,15 @@ stat_done

if [[ $HOSTNAME ]]; then stat_busy "Setting Hostname: $HOSTNAME" - echo "$HOSTNAME">| /proc/sys/kernel/hostname&& stat_done || stat_fail + echo "$HOSTNAME">| /proc/sys/kernel/hostname + stat_done fi

# Flush old locale settings and set user defined locale stat_busy "Setting Locale: ${LOCALE:=en_US}" - echo "export LANG=$LOCALE"> /etc/profile.d/locale.sh&& -chmod 0755 /etc/profile.d/locale.sh&& stat_done || stat_fail + echo "export LANG=$LOCALE"> /etc/profile.d/locale.sh + chmod 0755 /etc/profile.d/locale.sh +stat_done

if [[ ${LOCALE,,} =~ utf ]]; then stat_busy "Setting Consoles to UTF-8 mode" @@ -282,7 +289,7 @@ stat_busy "Saving dmesg Log" else install -Tm 0644<( dmesg ) /var/log/dmesg.log fi -(( $? == 0 ))&& stat_done || stat_fail +stat_done

run_hook sysinit_end

-- 1.7.1

Use correct patterns and get rid of the /dev/null redirection workaround needed when trying to remove '..'. --- rc.sysinit | 2 +- 1 files changed, 1 insertions(+), 1 deletions(-) diff --git a/rc.sysinit b/rc.sysinit index bfaaf1b..6af105f 100755 --- a/rc.sysinit +++ b/rc.sysinit @@ -227,7 +227,7 @@ RANDOM_SEED=/var/lib/misc/random-seed cp $RANDOM_SEED /dev/urandom stat_busy "Removing Leftover Files" - rm -rf /etc/{nologin,shutdownpid} /forcefsck /tmp/* /tmp/.* /var/run/daemons &>/dev/null + rm -rf /etc/{nologin,shutdownpid} /forcefsck /tmp/* /tmp/.[^.]* /tmp/..?* /var/run/daemons [[ ! -L /var/lock ]] && rm -rf /var/lock/* if [[ ! -L /var/run && -d /var/run ]]; then find /var/run/ \! -type d -delete -- 1.7.1

This avoids trouble because of stale /run/daemons/* files. --- rc.single | 3 +++ 1 files changed, 3 insertions(+), 0 deletions(-) diff --git a/rc.single b/rc.single index 7a87c72..3527a99 100755 --- a/rc.single +++ b/rc.single @@ -23,6 +23,9 @@ if [[ $PREVLEVEL != N ]]; then # Start/trigger UDev, load MODULES and settle UDev udevd_modprobe single + + # Removing leftover files + remove_leftover fi run_hook single_end -- 1.7.1

Avoid overriding error condition in case any custom stat_fail returns false. --- functions | 2 +- 1 files changed, 1 insertions(+), 1 deletions(-) diff --git a/functions b/functions index a3c3c5e..2bd6893 100644 --- a/functions +++ b/functions @@ -431,7 +431,7 @@ remove_leftover() { # Keep {x,k,g}dm happy with xorg mkdir -m 1777 /tmp/.{X11,ICE}-unix trap - ERR - (( error )) && stat_fail || stat_done + (( error == 0 )) && stat_done || stat_fail return $error } -- 1.7.1

--- functions | 10 +++++----- 1 files changed, 5 insertions(+), 5 deletions(-) diff --git a/functions b/functions index 77387cb..722098d 100644 --- a/functions +++ b/functions @@ -512,13 +512,13 @@ set_consolefont() { setfont ${CONSOLEMAP:+-m ${CONSOLEMAP}} \ $CONSOLEFONT -C ${i} &>/dev/null done - if (( $? )); then - false - elif [[ $CONSOLEMAP ]]; then - cat <<"EOF" >>/etc/profile.d/locale.sh + if (( $? )); then + false + elif [[ $CONSOLEMAP ]]; then + cat <<"EOF" >>/etc/profile.d/locale.sh if [ "$CONSOLE" = "" -a "$TERM" = "linux" -a -t 1 ]; then printf "\033(K"; fi EOF - fi + fi (( $? == 0 )) && stat_done || stat_fail } -- 1.7.1

--- functions | 4 ++-- 1 files changed, 2 insertions(+), 2 deletions(-) diff --git a/functions b/functions index bdb8529..1414ac8 100644 --- a/functions +++ b/functions @@ -39,7 +39,7 @@ calc_columns () { if [[ -t 1 ]]; then SAVE_POSITION="\e[s" RESTORE_POSITION="\e[u" - DEL_TEXT="\e[$(($STAT_COL+4))G" + DEL_TEXT="\e[$((STAT_COL+4))G" else SAVE_POSITION="" RESTORE_POSITION="" @@ -158,7 +158,7 @@ status() { shift "$@" &>/dev/null local ret=$? - (( $ret == 0 )) && stat_done || stat_fail + (( ret == 0 )) && stat_done || stat_fail return $ret } -- 1.7.1

The rules should be: * Use quotes when literal strings are involved. * Use quotes when ever using a substitution which might produce blanks as a possitional parameter unless word splitting is intended. * Don't use quotes for substitutions where _clearly_ not needed. That is within '[[ ]]', assignments and 'case ... in' (where no word splitting is performed) and also when using local variables which _obviously_ don't contain blanks. --- functions | 50 +++++++++++++++++++++++++------------------------- rc.sysinit | 10 +++++----- 2 files changed, 30 insertions(+), 30 deletions(-) diff --git a/functions b/functions index 722098d..ddd9585 100644 --- a/functions +++ b/functions @@ -20,9 +20,9 @@ calc_columns () { USECOLOR="" elif [[ -t 0 ]]; then # stty will fail when stdin isn't a terminal - STAT_COL="$(stty size)" + STAT_COL=$(stty size) # stty gives "rows cols"; strip the rows number, we just want columns - STAT_COL="${STAT_COL##* }" + STAT_COL=${STAT_COL##* } elif tput cols &>/dev/null; then # is /usr/share/terminfo already mounted, and TERM recognized? STAT_COL=$(tput cols) @@ -50,7 +50,7 @@ calc_columns () { calc_columns # disable colors on broken terminals -TERM_COLORS="$(tput colors 2>/dev/null)" +TERM_COLORS=$(tput colors 2>/dev/null) if (( $? != 3 )); then case $TERM_COLORS in *[!0-9]*) USECOLOR="";; @@ -76,16 +76,16 @@ fi # set colors if [[ $USECOLOR = [yY][eE][sS] ]]; then if tput setaf 0 &>/dev/null; then - C_CLEAR="$(tput sgr0)" # clear text - C_MAIN="${C_CLEAR}$(tput bold)" # main text - C_OTHER="${C_MAIN}$(tput setaf 4)" # prefix & brackets - C_SEPARATOR="${C_MAIN}$(tput setaf 0)" # separator - C_BUSY="${C_CLEAR}$(tput setaf 6)" # busy - C_FAIL="${C_MAIN}$(tput setaf 1)" # failed - C_DONE="${C_MAIN}" # completed - C_BKGD="${C_MAIN}$(tput setaf 5)" # backgrounded - C_H1="${C_MAIN}" # highlight text 1 - C_H2="${C_MAIN}$(tput setaf 6)" # highlight text 2 + C_CLEAR=$(tput sgr0) # clear text + C_MAIN=${C_CLEAR}$(tput bold) # main text + C_OTHER=${C_MAIN}$(tput setaf 4) # prefix & brackets + C_SEPARATOR=${C_MAIN}$(tput setaf 0) # separator + C_BUSY=${C_CLEAR}$(tput setaf 6) # busy + C_FAIL=${C_MAIN}$(tput setaf 1) # failed + C_DONE=${C_MAIN} # completed + C_BKGD=${C_MAIN}$(tput setaf 5) # backgrounded + C_H1=${C_MAIN} # highlight text 1 + C_H2=${C_MAIN}$(tput setaf 6) # highlight text 2 else C_CLEAR="\e[m" # clear text C_MAIN="\e[;1m" # main text @@ -93,9 +93,9 @@ if [[ $USECOLOR = [yY][eE][sS] ]]; then C_SEPARATOR="\e[1;30m" # separator C_BUSY="\e[;36m" # busy C_FAIL="\e[1;31m" # failed - C_DONE="${C_MAIN}" # completed + C_DONE=${C_MAIN} # completed C_BKGD="\e[1;35m" # backgrounded - C_H1="${C_MAIN}" # highlight text 1 + C_H1=${C_MAIN} # highlight text 1 C_H2="\e[1;36m" # highlight text 2 fi fi @@ -198,7 +198,7 @@ have_daemon() { ck_autostart() { local d for d in "${DAEMONS[@]}"; do - [[ "$1" = ${d#@} ]] && return 1 + [[ $1 = ${d#@} ]] && return 1 done return 0 } @@ -247,11 +247,11 @@ get_pid() { # Check if PID-file $1 is still the active PID-file for command $2 ck_pidfile() { - if [[ -f "$1" ]]; then + if [[ -f $1 ]]; then local fpid ppid read -r fpid <"$1" - ppid=$(get_pid $2) - [[ "$fpid" = "$ppid" ]] && return 0 + ppid=$(get_pid "$2") + [[ $fpid = $ppid ]] && return 0 fi return 1 } @@ -279,7 +279,7 @@ stop_all_daemons() { local i for (( i=${#DAEMONS[@]}-1; i>=0; i-- )); do [[ ${DAEMONS[i]} = '!'* ]] && continue - ck_daemon ${DAEMONS[i]#@} || stop_daemon ${DAEMONS[i]#@} + ck_daemon "${DAEMONS[i]#@}" || stop_daemon "${DAEMONS[i]#@}" done } @@ -325,7 +325,7 @@ udevd_modprobe() { status "Loading Modules" modprobe -ab "${MODULES[@]}" status "Waiting for UDev uevents to be processed" \ - udevadm settle --timeout=${UDEV_TIMEOUT:-30} + udevadm settle --timeout="${UDEV_TIMEOUT:-30}" run_hook "$1_udevsettled" @@ -374,7 +374,7 @@ NETFS="nfs,nfs4,smbfs,cifs,codafs,ncpfs,shfs,fuse,fuseblk,glusterfs,davfs,fuse.g # Check local filesystems fsck_all() { - fsck -A -T -C$FSCK_FD -a -t "no${NETFS//,/,no},noopts=_netdev" $FORCEFSCK + fsck -A -T -C"$FSCK_FD" -a -t "no${NETFS//,/,no},noopts=_netdev" $FORCEFSCK return $? } @@ -486,7 +486,7 @@ if (( RC_FUNCTIONS_HOOK_FUNCS_DEFINED != 1 )); then add_hook() { [[ $1 && $2 ]] || return 1 - hook_funcs["$1"]+=" $2" + hook_funcs[$1]+=" $2" } run_hook() { @@ -509,8 +509,8 @@ set_consolefont() { [[ ${LOCALE,,} =~ utf ]] && CONSOLEMAP="" local i for i in /dev/tty[0-9]*; do - setfont ${CONSOLEMAP:+-m ${CONSOLEMAP}} \ - $CONSOLEFONT -C ${i} &>/dev/null + setfont ${CONSOLEMAP:+-m "${CONSOLEMAP}"} \ + "$CONSOLEFONT" -C ${i} &>/dev/null done if (( $? )); then false diff --git a/rc.sysinit b/rc.sysinit index 1516fba..08ad906 100755 --- a/rc.sysinit +++ b/rc.sysinit @@ -96,14 +96,14 @@ if [[ -f /etc/crypttab && $CS ]] && grep -q ^[^#] /etc/crypttab; then # $3 = password # $4 = options stat_append "${1}.." - local open=create a="$1" b="$2" failed=0 + local open=create a=$1 b=$2 failed=0 # Ordering of options is different if you are using LUKS vs. not. # Use ugly swizzling to deal with it. # isLuks only gives an exit code but no output to stdout or stderr. if $CS isLuks "$2" 2>/dev/null; then open=luksOpen - a="$2" - b="$1" + a=$2 + b=$1 fi case $3 in SWAP) @@ -179,7 +179,7 @@ declare -r FORCEFSCK run_hook sysinit_prefsck if [[ -x $(type -P fsck) ]]; then stat_busy "Checking Filesystems" - fsck_all >|${FSCK_OUT:-/dev/stdout} 2>|${FSCK_ERR:-/dev/stdout} + fsck_all >|"${FSCK_OUT:-/dev/stdout}" 2>|"${FSCK_ERR:-/dev/stdout}" declare -r fsckret=$? (( fsckret <= 1 )) && stat_done || stat_fail else @@ -261,7 +261,7 @@ else stat_done fi [[ $KEYMAP ]] && - status "Loading Keyboard Map: $KEYMAP" loadkeys -q $KEYMAP + status "Loading Keyboard Map: $KEYMAP" loadkeys -q "$KEYMAP" # Set console font if required set_consolefont -- 1.7.1

On Tue, Jul 12, 2011 at 10:55:19AM +0200, Kurt J. Bosch wrote:

Avoid overriding error condition in case any custom stat_fail returns false. --- functions | 2 +- 1 files changed, 1 insertions(+), 1 deletions(-)

diff --git a/functions b/functions index a3c3c5e..2bd6893 100644 --- a/functions +++ b/functions @@ -431,7 +431,7 @@ remove_leftover() { # Keep {x,k,g}dm happy with xorg mkdir -m 1777 /tmp/.{X11,ICE}-unix trap - ERR - (( error )) && stat_fail || stat_done + (( error == 0 )) && stat_done || stat_fail return $error }

-- 1.7.1

This is a bug fix patch on top of an unmerged patch...

Dave Reisner, 2011-07-12 12:19:

On Tue, Jul 12, 2011 at 10:58:36AM +0200, Kurt J. Bosch wrote:

...

The rules should be:

* Use quotes when literal strings are involved. * Use quotes when ever using a substitution which might produce blanks as a possitional parameter unless word splitting is intended. * Don't use quotes for substitutions where _clearly_ not needed. That is within '[[ ]]', assignments and 'case ... in' (where no word splitting is performed) and also when using local variables which _obviously_ don't contain blanks.

The RHS of a bash test is subject to glob matching and should almost always be quoted.

$ pattern=foo* $ [[ foobar = $pattern ]]; echo $? 0 $ [[ foobar = "$pattern" ]]; echo $? 1

So this isn't really black and white. Fortunately, this doesn't appear to be an issue.

Good catch. Fixed this in the new thread together with those below to avoid this will ever be forgotten.

Other comments inlined.

...

--- functions | 50 +++++++++++++++++++++++++------------------------- rc.sysinit | 22 +++++++++++----------- 2 files changed, 36 insertions(+), 36 deletions(-)

diff --git a/functions b/functions index 722098d..bdb8529 100644 --- a/functions +++ b/functions @@ -20,9 +20,9 @@ calc_columns () { USECOLOR="" elif [[ -t 0 ]]; then # stty will fail when stdin isn't a terminal - STAT_COL="$(stty size)" + STAT_COL=$(stty size) # stty gives "rows cols"; strip the rows number, we just want columns - STAT_COL="${STAT_COL##* }" + STAT_COL=${STAT_COL##* } elif tput cols&>/dev/null; then # is /usr/share/terminfo already mounted, and TERM recognized? STAT_COL=$(tput cols) @@ -50,7 +50,7 @@ calc_columns () { calc_columns

# disable colors on broken terminals -TERM_COLORS="$(tput colors 2>/dev/null)" +TERM_COLORS=$(tput colors 2>/dev/null) if (( $? != 3 )); then case $TERM_COLORS in *[!0-9]*) USECOLOR="";; @@ -76,16 +76,16 @@ fi # set colors if [[ $USECOLOR = [yY][eE][sS] ]]; then if tput setaf 0&>/dev/null; then - C_CLEAR="$(tput sgr0)" # clear text - C_MAIN="${C_CLEAR}$(tput bold)" # main text - C_OTHER="${C_MAIN}$(tput setaf 4)" # prefix& brackets - C_SEPARATOR="${C_MAIN}$(tput setaf 0)" # separator - C_BUSY="${C_CLEAR}$(tput setaf 6)" # busy - C_FAIL="${C_MAIN}$(tput setaf 1)" # failed - C_DONE="${C_MAIN}" # completed - C_BKGD="${C_MAIN}$(tput setaf 5)" # backgrounded - C_H1="${C_MAIN}" # highlight text 1 - C_H2="${C_MAIN}$(tput setaf 6)" # highlight text 2 + C_CLEAR=$(tput sgr0) # clear text + C_MAIN=${C_CLEAR}$(tput bold) # main text + C_OTHER=${C_MAIN}$(tput setaf 4) # prefix& brackets + C_SEPARATOR=${C_MAIN}$(tput setaf 0) # separator + C_BUSY=${C_CLEAR}$(tput setaf 6) # busy + C_FAIL=${C_MAIN}$(tput setaf 1) # failed + C_DONE=${C_MAIN} # completed + C_BKGD=${C_MAIN}$(tput setaf 5) # backgrounded + C_H1=${C_MAIN} # highlight text 1 + C_H2=${C_MAIN}$(tput setaf 6) # highlight text 2 else C_CLEAR="\e[m" # clear text C_MAIN="\e[;1m" # main text @@ -93,9 +93,9 @@ if [[ $USECOLOR = [yY][eE][sS] ]]; then C_SEPARATOR="\e[1;30m" # separator C_BUSY="\e[;36m" # busy C_FAIL="\e[1;31m" # failed - C_DONE="${C_MAIN}" # completed + C_DONE=${C_MAIN} # completed C_BKGD="\e[1;35m" # backgrounded - C_H1="${C_MAIN}" # highlight text 1 + C_H1=${C_MAIN} # highlight text 1 C_H2="\e[1;36m" # highlight text 2 fi fi @@ -198,7 +198,7 @@ have_daemon() { ck_autostart() { local d for d in "${DAEMONS[@]}"; do - [[ "$1" = ${d#@} ]]&& return 1 + [[ $1 = ${d#@} ]]&& return 1 done return 0 } @@ -247,11 +247,11 @@ get_pid() {

# Check if PID-file $1 is still the active PID-file for command $2 ck_pidfile() { - if [[ -f "$1" ]]; then + if [[ -f $1 ]]; then local fpid ppid read -r fpid<"$1" - ppid=$(get_pid $2) - [[ "$fpid" = "$ppid" ]]&& return 0 + ppid=$(get_pid "$2") + [[ $fpid = $ppid ]]&& return 0 fi return 1 } @@ -279,7 +279,7 @@ stop_all_daemons() { local i for (( i=${#DAEMONS[@]}-1; i>=0; i-- )); do [[ ${DAEMONS[i]} = '!'* ]]&& continue - ck_daemon ${DAEMONS[i]#@} || stop_daemon ${DAEMONS[i]#@} + ck_daemon "${DAEMONS[i]#@}" || stop_daemon "${DAEMONS[i]#@}" done }

@@ -325,7 +325,7 @@ udevd_modprobe() { status "Loading Modules" modprobe -ab "${MODULES[@]}"

status "Waiting for UDev uevents to be processed" \ - udevadm settle --timeout=${UDEV_TIMEOUT:-30} + udevadm settle --timeout="${UDEV_TIMEOUT:-30}"

Why does this warrant quoting? passing --timeout= is harmless. If a user sets anything but a number here it's a syntax error, and udevadm will, furthermore, ignore the argument that it doesn't understand when multiple words are assigned.

OK. You convinced me.

...

run_hook "$1_udevsettled"

@@ -374,7 +374,7 @@ NETFS="nfs,nfs4,smbfs,cifs,codafs,ncpfs,shfs,fuse,fuseblk,glusterfs,davfs,fuse.g

# Check local filesystems fsck_all() { - fsck -A -T -C$FSCK_FD -a -t "no${NETFS//,/,no},noopts=_netdev" $FORCEFSCK + fsck -A -T -C"$FSCK_FD" -a -t "no${NETFS//,/,no},noopts=_netdev" $FORCEFSCK return $? }

@@ -486,7 +486,7 @@ if (( RC_FUNCTIONS_HOOK_FUNCS_DEFINED != 1 )); then

add_hook() { [[ $1&& $2 ]] || return 1 - hook_funcs["$1"]+=" $2" + hook_funcs[$1]+=" $2" }

run_hook() { @@ -509,8 +509,8 @@ set_consolefont() { [[ ${LOCALE,,} =~ utf ]]&& CONSOLEMAP="" local i for i in /dev/tty[0-9]*; do - setfont ${CONSOLEMAP:+-m ${CONSOLEMAP}} \ - $CONSOLEFONT -C ${i}&>/dev/null + setfont ${CONSOLEMAP:+-m "${CONSOLEMAP}"} \ + "$CONSOLEFONT" -C ${i}&>/dev/null done if (( $? )); then false diff --git a/rc.sysinit b/rc.sysinit index 1516fba..839a787 100755 --- a/rc.sysinit +++ b/rc.sysinit @@ -96,14 +96,14 @@ if [[ -f /etc/crypttab&& $CS ]]&& grep -q ^[^#] /etc/crypttab; then # $3 = password # $4 = options stat_append "${1}.." - local open=create a="$1" b="$2" failed=0 + local open=create a=$1 b=$2 failed=0 # Ordering of options is different if you are using LUKS vs. not. # Use ugly swizzling to deal with it. # isLuks only gives an exit code but no output to stdout or stderr. if $CS isLuks "$2" 2>/dev/null; then open=luksOpen - a="$2" - b="$1" + a=$2 + b=$1 fi case $3 in SWAP) @@ -123,13 +123,13 @@ if [[ -f /etc/crypttab&& $CS ]]&& grep -q ^[^#] /etc/crypttab; then fi if (( _overwriteokay == 0 )); then false - elif $CS -d /dev/urandom $4 $open "$a" "$b">/dev/null; then + elif $CS -d /dev/urandom "$4" $open "$a" "$b">/dev/null; then stat_append "creating swapspace.." - mkswap -f -L $1 /dev/mapper/$1>/dev/null + mkswap -f -L $1 /dev/mapper/"$1">/dev/null fi;; ASK) printf "\nOpening '$1' volume:\n" - $CS $4 $open "$a" "$b"< /dev/console;; + $CS "$4" $open "$a" "$b"< /dev/console;; /dev*) local ckdev=${3%%:*} local cka=${3#*:} @@ -151,13 +151,13 @@ if [[ -f /etc/crypttab&& $CS ]]&& grep -q ^[^#] /etc/crypttab; then # cka is numeric: cka=offset, ckb=length dd if=${ckdev} of=${ckfile} bs=1 skip=${cka} count=${ckb}>/dev/null 2>&1;; esac - $CS -d ${ckfile} $4 $open "$a" "$b">/dev/null + $CS -d ${ckfile} "$4" $open "$a" "$b">/dev/null dd if=/dev/urandom of=${ckfile} bs=1 count=$(stat -c %s ${ckfile}) conv=notrunc>/dev/null 2>&1 rm ${ckfile};; /*) - $CS -d "$3" $4 $open "$a" "$b">/dev/null;; + $CS -d "$3" "$4" $open "$a" "$b">/dev/null;; *) - echo "$3" | $CS $4 $open "$a" "$b">/dev/null;; + echo "$3" | $CS "$4" $open "$a" "$b">/dev/null;; esac if (( $? )); then failed=1 @@ -179,7 +179,7 @@ declare -r FORCEFSCK run_hook sysinit_prefsck if [[ -x $(type -P fsck) ]]; then stat_busy "Checking Filesystems" - fsck_all>|${FSCK_OUT:-/dev/stdout} 2>|${FSCK_ERR:-/dev/stdout} + fsck_all>|"${FSCK_OUT:-/dev/stdout}" 2>|"${FSCK_ERR:-/dev/stdout}" declare -r fsckret=$? (( fsckret<= 1 ))&& stat_done || stat_fail else @@ -261,7 +261,7 @@ else stat_done fi [[ $KEYMAP ]]&& - status "Loading Keyboard Map: $KEYMAP" loadkeys -q $KEYMAP + status "Loading Keyboard Map: $KEYMAP" loadkeys -q "$KEYMAP"

KEYMAP might be multiple words, intentionally, to load multiple keymaps. We cannot quote this.

https://bugs.archlinux.org/task/23373

Oops, thank you for clarifying this.

...

# Set console font if required set_consolefont -- 1.7.1

-- Kurt