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