avoiding bash pitfalls and code smells

about me

  • My name is Chris Down
  • Sysadmin at Regent Markets Group
  • Sysadmin/programmer at Initiatives of Change
  • Tech Lead at zenu.ca
  • 5th most trusted user in the world on U&LSE
  • Actively involved with development of bash

why so prone to errors?

  • Lacks common programming constructs
  • Integer only return values
  • Most passing of data requires parsing
  • Manipulates externally documented tools
  • Questionable defaults
  • In some cases, unclear syntax

do you really want to use bash?

  • Acting as a wrapper for simple IPC
  • Manipulating file hierarchies
  • Launching other programs
  • ...that's about it.

"my data will always look like X"

for arg in $*; do
# in $@# in "$*"# in "$@" printf '<%s>\n' "$arg" done
# Args: "i love" "buttermilk"
# With args: "i love" "buttermilk"
# for arg in $*<i> <love> <buttermilk> # for arg in $@ <i> <love> <buttermilk> # for arg in "$*" <i love buttermilk> # for arg in "$@" <i love> <buttermilk>
$ var=( foo bar baz )$ echo "${var[@]}"foo bar baz$ echo "${var[*]}"
foo bar baz
$ IFS=!
$ echo "${var[@]}"
foo bar baz$ echo "${var[*]}"foo!bar!baz
for file in *; do    rm "$file"done
# With a file called -foobar:rm: invalid option -- 'o'
Try 'rm --help' for more information.
for file in *; do    rm "./$file"    # rm -- "$file"done
drwx------ 2 root root 4096 Mar 15 03:20 /root_in
#!/bin/bash# You want to archive user dirs to a root-only dir.# This is allowed to be used with sudo without a password.# Is this safe (and forgive the convoluted example)?
find /user_out -type d | head -5 | while read file; do mv "$file" /root_in done
# Files: "i love spaces" "i really do"
# mv "/user_out/i love spaces" "/root_in"
# mv "/user_out/i really do" "/root_in"
chris@foo$ mkdir -p $'/srv/user_in/x\n/sbin/init'
chris@foo$ sudo user_mover
mv: cannot stat "x": No such file or directory
chris@foo$ ls /sbin/init
ls: cannot access /sbin/init: No such file or directory
# Enjoy the next reboot.

# STDOUT and STDERR to /dev/nullcommand 2>&1 >/dev/null
"Redirect everything on FD 2 (STDERR) to the same location as FD 1 (STDOUT) is going, and send everything on FD 1 to /dev/null.
$ { echo 1; echo 2 >&2; } 2>&1 >/dev/null2$ { echo 1; echo 2 >&2; } >/dev/null 2>&1$
#!/bin/bashweek=$(date +%W) # Week number of year {00..53}
# Every fourth week, we shift rotarota=$(( week % 4 ))
bash: 09: value too great for base (error token is "09")
# Convert to base 10rota=$(( 10#week % 4 ))
# Strip leading zeroesweek="${week##0}"
# For this particular case only, with GNU dateweek="$(date +%-w)"
if [[ "$1" == calum ]]; then view "haggis.jpg" view "deep_fried_mars_bar.jpg"fi
sh: 3: [[: not found# we lost calum's lunch. :-(

Always use /bin/bash as the interpreter unless you are 100% sure that what you wrote is POSIX!

if [ -n `ps aux | grep daemon_universal_wrapper | grep -v grep` ]; then    # daemon_universal wrapper is running    [...]

always use exit codes instead of parsing where possible!

# wtf is "["?if [

"[" is a command like any other

$ [ -z foo ] ; echo $?1$ pgrep -f daemon_universal_wrapper0
if pgrep -f daemon_universal_wrapper >/dev/null; then    # daemon_universal_wrapper is running    [...]

use -u/nounset where possible

set -u
echo "$x"
bash: x: unbound variable

use set -e for short scripts (maybe)

set -e has a lot of caveats, but it can be useful for short scripts that should throw no errors anywhere. Beware caveats!

  • Maybe: 8 lines merely executing commands one after the other
  • No: 50 line script

General guidelines

  • Think about whether bash is really the best tool
  • Quote every expansion
  • Stop using "[" as a sledgehammer
  • Think about portability
  • Think about how parsing could mangle your data

$ echo "Thanks for your time!!"bash: !!: event not found