Robust Scripts · about 45 min

The classic pitfalls

Why this matters

Most broken shell scripts are broken in the same dozen ways. The bugs hide because the script works on the author's test files (no spaces, nothing empty, nothing starting with a dash) and fails on the first real-world input, sometimes destructively. The tool shellcheck is a static analyzer that recognizes these patterns and gives each one a code such as SC2086; its wiki page for each code is an excellent explanation. shellcheck is not installed in this sandbox, but you should install it wherever you write scripts (apt install shellcheck, dnf install ShellCheck, brew install shellcheck) and run it on everything. This lesson teaches the most important warnings by name so you recognize them, understand why they are bugs, and avoid them without a tool.

Each pitfall below comes back to something you learned in module 7: bash expands a line before running it, and unquoted expansions go through word splitting and globbing.

SC2086: unquoted variables

file="my notes.txt"
rm $file          # rm receives two arguments: "my" and "notes.txt"
rm "$file"        # rm receives one argument: "my notes.txt"

An unquoted $file is split on spaces, tabs and newlines (the characters in IFS), and each piece is then treated as a glob pattern: a value of * expands to every file in the directory.

$ msg="*"
$ echo $msg
cleanup.sh  notes.txt  sample dir
$ echo "$msg"
*

Rule: quote every expansion, "$var", "$(cmd)", "$@", unless you can name the specific reason you want splitting. Exceptions where quotes are not needed: the right side of an assignment (a=$b), inside [[ ]], and in case $x in. Quoting there anyway is harmless.

SC2045 / SC2012: parsing ls

for f in $(ls *.txt); do ...      # broken
for f in *.txt; do ...            # right

ls prints names as text for humans, one per line or in columns, sometimes with ? replacing odd characters. $(ls) then re-splits that text on whitespace, so old report.txt becomes two loop iterations, old and report.txt, neither of which exists. A glob produces a list of real names directly, with no text stage in between: each name is one word, whatever it contains.

When nothing matches, a glob stays as the literal pattern (*.txt), so either check [ -e "$f" ] || continue in the loop or enable shopt -s nullglob (module 14). For recursive searches, use find with -print0 (module 14), never ls -R.

SC2164: cd without a check

cd "$build_dir"
rm -rf ./*

If cd fails, the rm runs in the current directory. Always:

cd "$build_dir" || exit 1

Or do the work in a subshell, ( cd "$build_dir" && make ), so the main script's directory never changes (module 13).

SC2115: rm -rf with a variable that may be empty

rm -rf "$prefix/"*        # prefix empty → rm -rf /*
rm -rf "${prefix:?}/"*    # stops with "prefix: parameter null or not set"

${var:?} makes the shell abort if the variable is unset or empty. set -u catches unset, but not empty. Steam for Linux once shipped a script with rm -rf "$STEAMROOT/"*, and when $STEAMROOT came out empty, it deleted users' entire home directories. Validate paths before destructive commands.

SC2155: declare and assign separately

local out=$(cmd)     # status of cmd is lost; local always succeeds
local out
out=$(cmd)           # status of cmd is the status of this line

The same for export and readonly. Under set -e the first form silently continues past a failure.

SC2162: read without -r (and IFS=)

while read line; do ...          # mangles backslashes and edge spaces
while IFS= read -r line; do ...  # reads the line exactly

Without -r, read treats backslash as an escape character: C:\temp becomes C:temp. Without IFS=, leading and trailing spaces are stripped. You learned this in the loops module; shellcheck flags every read without -r.

SC2002: useless use of cat

cat access.log | grep 404      # an extra process and a pipe for nothing
grep 404 access.log            # same result
< access.log grep 404          # or redirect, if you like reading left to right

Not a correctness bug, but costs a process, and it hides the file name from tools like wc and grep -l that would otherwise print it.

SC2181: checking $? indirectly

mycmd
if [ $? -ne 0 ]; then ...     # fragile, and fatal under set -e
if ! mycmd; then ...          # direct

SC2006, SC2034, SC2154: smaller signals of trouble

More traps worth knowing

A checklist before you trust a script

  1. Every expansion quoted, or a reason it is not.
  2. No $(ls ...), no for f in $(find ...); globs or find -print0.
  3. Every cd checked; destructive paths validated (${var:?}).
  4. read always IFS= read -r.
  5. Errors to stderr with a non-zero status; strict mode on.
  6. Tested with a file name containing a space, a dash at the start, and an empty input.
  7. shellcheck clean, when you have it.

Common mistakes

This whole lesson is a list of them. The meta-mistake is testing only with tidy inputs. Keep a nasty test directory around: touch -- 'a b' '-n' '*' "$(printf 'new\nline')" and run your scripts against it.

Exercises

A quiz to spot the bugs, a quiz on SC2155, and a hands-on repair of a script that fails on almost every pitfall above. The repaired script is tested against the checker's own directories, so fix the causes, not the symptoms you see on the sample.

Recap

Going further: the shellcheck wiki has one page per code (search for "shellcheck SC2086"). The "Bash Pitfalls" page on Greg's Wiki (mywiki.wooledge.org) lists over 60 more, with explanations; read the first ten.

Exercises

Exercise 1 · quiz

Spot the bugs

Which of these lines contain a real bug (wrong behavior for some ordinary input, not just bad style)? Assume $file, $dir and $f may contain spaces. Select all that apply.

Answer
  • rm $file

  • for f in $(ls *.txt); do wc -l "$f"; done

  • while read line; do echo "$line"; done < input.txt

rm $file is split into words (and globbed) when the name has spaces: SC2086. $(ls *.txt) turns names back into text and re-splits them on spaces: SC2045 and SC2046. read without -r eats backslashes and, without IFS=, strips leading and trailing spaces: SC2162. The cd line is the correct way to write it. cat "$f" | grep error works; it is only a wasted process ("useless use of cat", SC2002): grep error "$f" is the same.

Exercise 2 · quiz

The swallowed failure

Under set -euo pipefail, what does this print?

f() {
  local out=$(false)
  echo "f continued"
}
f
echo "script continued"
Answer
  • f continued then script continued.

The status of local out=$(false) is the status of the local builtin, which succeeded in creating the variable, so the failure of false is lost and set -e never sees it. shellcheck calls this SC2155 ("declare and assign separately to avoid masking return values"). The fix is two lines: local out then out=$(false); the plain assignment carries the substitution's status. The same applies to export x=$(cmd) and readonly x=$(cmd).

Exercise 3 · task in a shell

Fix a buggy cleanup script

/work/cleanup.sh is meant to delete every file ending in .tmp directly inside the directory given as its argument, and then print removed N. It is riddled with classic pitfalls. Look at it (cat cleanup.sh), try it on the sample directory /work/sample dir, and fix it (nano cleanup.sh) so that:

  1. bash cleanup.sh DIR removes every *.tmp file directly in DIR, including names with spaces and names starting with -, leaves every other file alone, and prints exactly removed N.
  2. A directory with no .tmp files gives removed 0.
  3. A missing or empty argument, or one that is not a directory, prints usage: cleanup.sh DIR to stderr, exits with status 2 and deletes nothing.

The checker runs your script on fresh test directories of its own (with awkward names), so it must work in general, not only on the sample.

Hint 1

Never loop over $(ls ...). Loop over a glob instead: for f in "$dir"/*.tmp; do. Each match is one word, spaces and all.

Hint 2

If nothing matches, the glob stays as the literal text DIR/*.tmp; skip it with [ -f "$f" ] || continue.

Hint 3

Check the argument first: if [ -z "$dir" ] || [ ! -d "$dir" ]; then echo "usage: cleanup.sh DIR" >&2; exit 2; fi (use dir=${1:-}), and quote every "$f".

Reference solution
cat > cleanup.sh <<'EOF'
#!/bin/bash
# remove every .tmp file directly inside DIR and report how many were removed
dir=${1:-}
if [ -z "$dir" ] || [ ! -d "$dir" ]; then
  echo "usage: cleanup.sh DIR" >&2
  exit 2
fi
count=0
for f in "$dir"/*.tmp; do
  [ -f "$f" ] || continue
  rm -- "$f"
  count=$((count + 1))
done
echo "removed $count"
EOF