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. 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.
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
# Set console font if required set_consolefont -- 1.7.1