6
\$\begingroup\$

I'm trying to create a bash shell script which mounts a disk image file. Not only that but checks to see if the disk image already exists. Is there anyway I could improve my script? Currently my script looks like this:

#!/bin/bash
MOUNTPOINT="/myfilesystem"
if [ ! -d "${MOUNTPOINT}" ]; then
 if [ -e "${MOUNTPOINT}" ]; then
 echo "Mountpoint exists, but isn't a directory..."
 exit 1
 else
 sudo mkdir -p "${MOUNTPOINT}"
 fi
fi
if [ "$(echo ${MOUNTPOINT}/*)" != "${MOUNTPOINT}/*" ]; then
 echo "Mountpoint is not empty!"
 exit 1
fi
if mountpoint -q "${MOUNTPOINT}"; then
 echo "Already mounted..."
 exit 0
fi
Phrancis
20.5k6 gold badges69 silver badges155 bronze badges
asked Mar 14, 2017 at 17:40
\$\endgroup\$

2 Answers 2

1
\$\begingroup\$

The first conditional group can be reorganized to make it flatter and simpler:

if [ ! -e "${MOUNTPOINT}" ]; then
 sudo mkdir -p "${MOUNTPOINT}"
elif [ -d "${MOUNTPOINT}" ]; then
 echo "Mountpoint exists, but isn't a directory..."
 exit 1
fi

The exit 0 is pointless in the last condition, and as such, you could replace it with a one-liner using &&:

mountpoint -q "${MOUNTPOINT}" && echo "Already mounted..."
answered Apr 19, 2017 at 19:51
\$\endgroup\$
1
\$\begingroup\$
  • Error messages should be printed on standard error, like `echo "Chocolate missing!">&2
  • Internal variables should be lowercase, like mount_point. Don't worry if it ends up being the same string as a command like mountpoint; Bash handles that just fine (and your IDE should highlight them to show the difference).
  • The idiomatic Bash shebang line is #!/usr/bin/env bash (disclaimer: my answer)
  • sudo is generally discouraged within scripts, for at least two reasons:
    1. The target system may not have sudo. This is uncommon, but some use su instead.
    2. It makes it harder to automate the use of the script, since sudo may prompt for a password.
  • It is generally expected of a shell script that it will not have unintended side effects. Your script will create the mount point directory if it doesn't exist.
  • Instead of hard coding your mount point you can easily make the script ... scriptable:

    for mount_point
    do
     [the contents of your script]
    done
    

    this will loop through all the arguments you provide ("$@")

  • I personally prefer moving the then and do keywords to a separate line, to avoid having to use ; in my scripts.
answered May 19, 2017 at 20:19
\$\endgroup\$

Your Answer

Draft saved
Draft discarded

Sign up or log in

Sign up using Google
Sign up using Email and Password

Post as a guest

Required, but never shown

Post as a guest

Required, but never shown

By clicking "Post Your Answer", you agree to our terms of service and acknowledge you have read our privacy policy.

Start asking to get answers

Find the answer to your question by asking.

Ask question

Explore related questions

See similar questions with these tags.