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.
Oops, thank you for clarifying this.
# Set console font if required set_consolefont -- 1.7.1
-- Kurt