[arch-projects] [initscripts] [PATCH 00/13] The rag rug roll rest refurbished
A fix for the merged stuff, some little improvements and a bit of cosmetics. Kurt J. Bosch (13): rc.sysinit: fixup fsck refactoring functions: Use ck_autostart in stop_all_daemons() functions: Make in_array() non DAEMONS specific rc.sysinit: Fix /tmp/.* pattern functions/rc.sysinit: Refactor 'Removing Leftover Files' code rc.single: Call remove_leftover() when going single-user functions: Catch and report all errors in remove_leftover() functions: Refactor set_consolefont() functions: Correct set_consolefont() indentation functions: Remove useless '$' within '(( ))' functions: Unify whitespace inside '(( ))' Fix/unify quoting functions: Cosmetics: Refactor usage of local daemon variables functions | 97 +++++++++++++++++++++++++++++++++++------------------------ rc.single | 3 ++ rc.sysinit | 23 ++++---------- 3 files changed, 67 insertions(+), 56 deletions(-)
Revert to using in_array() instead of is_in_array() which actually doesn't exist. --- rc.sysinit | 2 +- 1 files changed, 1 insertions(+), 1 deletions(-) diff --git a/rc.sysinit b/rc.sysinit index 7357582..bfaaf1b 100755 --- a/rc.sysinit +++ b/rc.sysinit @@ -174,7 +174,7 @@ if [[ -f /etc/crypttab && $CS ]] && grep -q ^[^#] /etc/crypttab; then fi # Check filesystems -[[ -f /forcefsck ]] || is_in_array forcefsck $(< /proc/cmdline) && FORCEFSCK="-- -f" +[[ -f /forcefsck ]] || in_array forcefsck $(< /proc/cmdline) && FORCEFSCK="-- -f" declare -r FORCEFSCK run_hook sysinit_prefsck if [[ -x $(type -P fsck) ]]; then -- 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 <d@falconindy.com> --- 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
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
Move it into a function to allow clean error detection of all steps performed and also to be able to reuse this in rc.single. --- functions | 14 ++++++++++++++ rc.sysinit | 13 ++----------- 2 files changed, 16 insertions(+), 11 deletions(-) diff --git a/functions b/functions index 000ee0e..fdf30fd 100644 --- a/functions +++ b/functions @@ -417,6 +417,20 @@ mount_all() { mount -a -t "nosysfs,no${NETFS//,/,no}" -O no_netdev } +remove_leftover() { + stat_busy "Removing Leftover Files" + 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 + ln -s /run/daemons /var/run/daemons + fi + install -Tm 0664 -o root -g utmp <(:) /var/run/utmp + # Keep {x,k,g}dm happy with xorg + mkdir -m 1777 /tmp/.{X11,ICE}-unix + stat_done +} + bootlogd_stop() { [[ -f /run/bootlogd.pid ]] || return 0 touch /var/log/boot diff --git a/rc.sysinit b/rc.sysinit index 6af105f..1516fba 100755 --- a/rc.sysinit +++ b/rc.sysinit @@ -226,17 +226,8 @@ RANDOM_SEED=/var/lib/misc/random-seed status "Initializing Random Seed" \ cp $RANDOM_SEED /dev/urandom -stat_busy "Removing Leftover Files" - 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 - ln -s /run/daemons /var/run/daemons - fi - install -Tm 0664 -o root -g utmp <(:) /var/run/utmp - # Keep {x,k,g}dm happy with xorg - mkdir -m 1777 /tmp/.{X11,ICE}-unix -stat_done +# Remove leftover files +remove_leftover if [[ $HOSTNAME ]]; then stat_busy "Setting Hostname: $HOSTNAME" -- 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
Use a trap on 'ERR' to catch any error. Doing so introduces no side effects as long as we don't use 'set -E' and do a 'trap - ERR' before returning to restore what we had there before calling the function (if anything). This avoids reporting '[DONE]' in spite of actions were done partly only. Usecase: Some splash system hides the console messages, but is able to detect '[FAIL]' by overriding stat_fail(). --- functions | 6 +++++- 1 files changed, 5 insertions(+), 1 deletions(-) diff --git a/functions b/functions index fdf30fd..2bd6893 100644 --- a/functions +++ b/functions @@ -419,6 +419,8 @@ mount_all() { remove_leftover() { stat_busy "Removing Leftover Files" + local error=0 + trap 'error=1' ERR 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 @@ -428,7 +430,9 @@ remove_leftover() { install -Tm 0664 -o root -g utmp <(:) /var/run/utmp # Keep {x,k,g}dm happy with xorg mkdir -m 1777 /tmp/.{X11,ICE}-unix - stat_done + trap - ERR + (( error == 0 )) && stat_done || stat_fail + return $error } bootlogd_stop() { -- 1.7.1
Simplify conditionals a bit to make this more readable. Make this a bit more readable and uniform. --- functions | 8 +++----- 1 files changed, 3 insertions(+), 5 deletions(-) diff --git a/functions b/functions index 2bd6893..77387cb 100644 --- a/functions +++ b/functions @@ -506,22 +506,20 @@ set_consolefont() { [[ $CONSOLEFONT ]] || return 0 stat_busy "Loading Console Font: $CONSOLEFONT" #CONSOLEMAP in UTF-8 shouldn't be used - [[ $CONSOLEMAP && ${LOCALE,,} =~ utf ]] && CONSOLEMAP="" + [[ ${LOCALE,,} =~ utf ]] && CONSOLEMAP="" local i for i in /dev/tty[0-9]*; do setfont ${CONSOLEMAP:+-m ${CONSOLEMAP}} \ $CONSOLEFONT -C ${i} &>/dev/null done if (( $? )); then - stat_fail + 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 - stat_done - else - stat_done fi + (( $? == 0 )) && stat_done || stat_fail } # Source additional functions at the end to allow overrides -- 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 722098d..9a44ab5 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
--- functions | 2 +- 1 files changed, 1 insertions(+), 1 deletions(-) diff --git a/functions b/functions index 9a44ab5..37ceb27 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="" -- 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. * Use quotes on the right hand side within '[[ = ]]' unless pattern matching is intended. * Don't use quotes for substitutions where not needed as in assignments. --- functions | 50 +++++++++++++++++++++++++------------------------- rc.sysinit | 8 ++++---- 2 files changed, 29 insertions(+), 29 deletions(-) diff --git a/functions b/functions index 37ceb27..93b7ab7 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 @@ -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 } @@ -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 } @@ -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..fe1b7e6 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 -- 1.7.1
--- functions | 11 ++++++----- 1 files changed, 6 insertions(+), 5 deletions(-) diff --git a/functions b/functions index 93b7ab7..8e31b79 100644 --- a/functions +++ b/functions @@ -196,9 +196,9 @@ have_daemon() { # Check if $1 is started at boot ck_autostart() { - local d - for d in "${DAEMONS[@]}"; do - [[ $1 = "${d#@}" ]] && return 1 + local daemon + for daemon in "${DAEMONS[@]}"; do + [[ $1 = "${daemon#@}" ]] && return 1 done return 0 } @@ -276,10 +276,11 @@ stop_all_daemons() { done # Shutdown daemons in reverse order - local i + local i daemon for (( i=${#DAEMONS[@]}-1; i>=0; i-- )); do [[ ${DAEMONS[i]} = '!'* ]] && continue - ck_daemon "${DAEMONS[i]#@}" || stop_daemon "${DAEMONS[i]#@}" + daemon=${DAEMONS[i]#@} + ck_daemon "$daemon" || stop_daemon "$daemon" done } -- 1.7.1
On Tue, Jul 12, 2011 at 6:10 PM, Kurt J. Bosch <kjb-temp-2009@alpenjodel.de> wrote:
A fix for the merged stuff, some little improvements and a bit of cosmetics.
Merged most of this stuff. Except for what I noted below:
Kurt J. Bosch (13): rc.sysinit: fixup fsck refactoring functions: Use ck_autostart in stop_all_daemons() functions: Make in_array() non DAEMONS specific rc.sysinit: Fix /tmp/.* pattern functions/rc.sysinit: Refactor 'Removing Leftover Files' code rc.single: Call remove_leftover() when going single-user functions: Catch and report all errors in remove_leftover()
As discussed before, I think this is too clever. As much as possible, I want the meaning of our code to be obvious even to someone who are not familiar with bash.
functions: Refactor set_consolefont() functions: Correct set_consolefont() indentation
I don't see that these last two patches are making an improvement, so dropped them.
functions: Remove useless '$' within '(( ))' functions: Unify whitespace inside '(( ))' Fix/unify quoting functions: Cosmetics: Refactor usage of local daemon variables
functions | 97 +++++++++++++++++++++++++++++++++++------------------------ rc.single | 3 ++ rc.sysinit | 23 ++++---------- 3 files changed, 67 insertions(+), 56 deletions(-)
Thanks! Tom
participants (2)
-
Kurt J. Bosch
-
Tom Gundersen