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
2 Answers 2
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..."
- 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 likemountpoint; 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) sudois generally discouraged within scripts, for at least two reasons:- The target system may not have
sudo. This is uncommon, but some usesuinstead. - It makes it harder to automate the use of the script, since
sudomay prompt for a password.
- The target system may not have
- 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] donethis will loop through all the arguments you provide (
"$@")- I personally prefer moving the
thenanddokeywords to a separate line, to avoid having to use;in my scripts.