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
