Bash Script Coding Rules

Rules

• Shebang is #!/bin/bash or #!/bin/sh. No whitespace between ! and the path. #! /bin/bash or #! /bin/sh is wrong, relative paths are wrong, only the name of the binary (#!bash) is even more wrong. If you're not certain that your script will run in sh or dash, use #!/bin/bash for good measure.

• All scripts must include set -e at the top. This terminates the script if any command returns an error.

  There are situations where a command can return a failure and you wish to ignore it. Double-check if that case really applies to you. Then check again. If you still think it does, use command || /bin/true. If you have to do that all over the place, you're doing it wrong. No, skipping set -e is not a solution then: You'll have to write your script more defensively. That means, add checks and only run those commands if you expect them to succeed.

• All scripts must include set -u at the top. This terminates the script on read access to undefined variables.

  The rationale behind this rule is that rm -rf $ROTPATH/* expands to rm -rf /* if you meant to type $ROOTPATH there. You do not want to find that bug on your production database. You'd rather bash told you, and this is what set -u gives you.

  From time to time though, you need to access an undefined variable, probably in order to find out if it's defined or not (command-line argumentés, for instance). Then these come in handy:

  • ${VAR:=default} returns the variable if defined, otherwise sets it to default and then returns that.

  • ${VAR:-default} returns the variable if defined or else default, without defining the variable if it's undefined.

  I tend to prefer the :- form. An appearance of := is usually a sign that you should've initialized that variable somewhere earlier in your script.

  Note that the default can also be the empty string: ${VAR:-} is perfectly fine. This is still better than omitting set -u and letting the shell just default to the empty string, because by making this explicit, you get to decide that this can be empty. You won't do that inside an rm -rf (unless you're a bad person).

• Consider whether or not your script should use set -x so it displays commands as it executes them. This is usually desirable in build scripts.

• Running binaries or sourcing scripts with relative paths is brittle and should be avoided. It breaks when the user runs the script from within another directory. Use absolute paths instead, or at the very least cd into the correct place.

• Always enclose variables that may contain whitespace (especially, user-controlled variables like command line arguments) in quotes to prevent the whitespace from doing weird things. So, this is bad:

  MYPATH=$1
  cd $MYPATH
  

  If MYPATH were to contain whitespace, this fails:

  # mkdir "/tmp/white space"
  # /tmp/test.sh "/tmp/white space"
  /tmp/test.sh: line 6: cd: /tmp/white: No such file or directory

  So instead, write this as:

  MYPATH="$1"
  cd "$MYPATH"
  

• if statements:

  if [ "${FEATURE_ENABLED:-false}" = "true" ] && [ "`whoami`" = "root" ]; then
      if apt-get update; then
          echo "Cool, we successfully updated Apt's package lists!"
      fi
  else
      echo "Our awesome cool feature is disabled or we don't have permissions :("
  fi
  

  • Space between if and [. if[ is an error and your script is broken (command if[ does not exist).

  • Space between [ and "$VAR". if ["$VAR" is an error and your script is broken (command [something does not exist).

  • $VAR inside "", so if the variable's value is the empty string "" or contains whitespace, your if statement doesn't break.

  • If you have multiple conditions, use && and || with multiple [ ] constructs instead of -a and -o. It's easier to read for programmers and I found -a and -o don't work reliably.

  • Use ${VAR:-default} to define a default value, so your script doesn't get killed by set -u if the variable can be undefined here.

  • Space around the = operator. It needs to be a word of its own.

  • Only a single = for comparisons. == is invalid in [ ] checks.

  • Space before ]. This also needs to be its own word.

  • No space after ], put the semicolon there directly.

  • Space after ;.

  • then on the same line as if

  • If if [ $? = 0 ]; then appears in your code, you're doing it wrong. Use if command; then instead.

• If you're using non-standard tools, check if they're installed. Such a check can be done using which:

  for tool in qemu-img guestfish guestmount debootstrap virt-install; do
      if ! which "$tool" >/dev/null; then
          echo "$tool is missing -> abort"
          exit 4
      fi
  done
  

• If you do something along the lines of PINGOUT="`ping -c5 "${HOST:-8.8.8.8}"`", be sure to close all'a dem "" correctly.

• No running things in the background using & without calling wait somewhere.

  • No, I won't grant you an exception.

  • No, you're not special.

  • Especially not if you're writing an init script. Use start-stop-daemon instead. If your distro doesn't have that, well, switch to one that does or use systemd. It's probably way easier anyway, init scripts are horrible.

  This is what it should look like:

  # Run a bunch'a jobs in parallel
  for job in $JOBS; do
      run_job "$job" &
  done
  
  # Wait for all of them to be finished
  wait
  

• Take care when redirecting both stdout and stderr because order matters. This works:

  apt-get update >/dev/null 2>&1
  

  This does not:

  apt-get update 2>&1 >/dev/null
  

  You can compare this to a programming language. Suppose you have two variables that you wish to change to the same value. This works:

  stdout = "/dev/null"
  stderr = stdout

  This does not:

  stderr = stdout
  stdout = "/dev/null"

  In the latter case, "/dev/null" is only written into the stdout variable, not into stderr. Bash internally does the same thing when you use >, only with file descriptors.

• Be aware that apt-get update &>/dev/null only works in bash, so have #!/bin/bash in your shebang if you use it.

• No indented multiline strings. Heck, no multiline strings at all. Use a here-document instead.

• If writing here-documents, put the <<EOF marker at the front of the line. It's easier to see this way. If your here-document is itself a bash script containing other here-documents, the EOF text may be anything you choose:

  <<THISWORKSTOOYAAY cat
  here-documents are pretty cool!
  THISWORKSTOOYAAY
  

• If you need to shell out to a subscript (e.g. if you want to run something complex inside a chroot or on a remote host), render it first using a here-document. Those can also contain variables:

  THIS_OTHER_PACKAGE="sysstat"
  
  <<EOF cat >"$MNT/install.sh"
  #!/bin/bash
  
  set -e     # Yes, i really do mean it when I say "all scripts"
  set -u
  
  apt-get install htop iftop iotop "$THIS_OTHER_PACKAGE"
  EOF
  
  chmod +x "$MNT/install.sh"
  chroot "$MNT" /install.sh
  

  Conveniently, this also works across SSH:

  THIS_OTHER_PACKAGE="sysstat"
  
  <<EOF ssh remote-host.somewhere.com /bin/bash -e -u
  apt-get install htop iftop iotop "$THIS_OTHER_PACKAGE"
  EOF
  

  Note though that variables are replaced before the content is written into the document, so if you want to use an actual variable in the document, you need to escape its $ sign. Furthermore, you'll need to escape backticks.

  #!/bin/bash
  
  set -e
  set -u
  
  <<EOF ssh remote-host.somewhere.com /bin/bash -e -u
  MYHOSTNAME="\`hostname --fqdn\`"
  echo "Oh hai, my name is \$MYHOSTNAME!"
  EOF
  

  If you're not sure what I mean, try this script with a simple $ instead of \$ and you'll see.

• Keep line length reasonable. I'm not saying it has to be 80 chars, but you should definitely manage to stay within 140.

Example

Here's an example script, outlining most of the rules above and it even contains a simple but elegant argument parser:

#!/bin/bash

set -e
set -u

# default server
SERVER="8.8.8.8"

while [ -n "${1:-}" ]; do
    case "$1" in
        -h|--help)
            echo "Usage: $0 [--mtr] [--server <server>]"
            echo
            echo "Options:"
            echo " -h --help             This help text"
            echo "    --mtr              Use mtr instead of ping"
            echo " -s --server           Server to connect to [$SERVER]"
            exit 0
            ;;

        --mtr)
            MTR=true
            ;;

        -s|--server)
            SERVER="$2"
            shift       # params with args need an extra shift
            ;;

        *)
            echo "Unknown option $1, see --help"
            exit 3
    esac
    shift
done

if [ "${MTR:-false}" = "true" ]; then
    if ! which mtr >/dev/null; then
        echo "mtr is missing -> abort"
        exit 4
    fi
    mtr "$SERVER"
else
    ping "$SERVER"
fi