avoiding bash pitfalls and code smells

about me

why so prone to errors?

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 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

$ { 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)"

#!/bin/sh
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

#!/bin/bash
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