I am trying to write is_device_mounted script, which in turn will serve a greater purpose in my home Linux system.

It does not even have an error reporting function included and as you can see, I have made it clean for readers. My intention here is to review the code for general Linux. But if you are on a *BSD, I would appreciate your feedback too!

The first version of the script follows:


set -eu

is_empty_string() { [ -z "${1}" ]; }
sanitize_device_string() { printf '%s' "${1}" | grep '^/dev/' | head -n 1; }
is_block_device() { [ -b "${1}" ]; }
is_device_uuid_identified() { printf '%s' "${1}" | grep -F '/by-uuid/'; }
translate_uuid_to_device_name() { readlink -f -n /dev/disk/by-uuid/"${1}"; }

    if [ -n "${device_name}" ]; then
        # 1. basic regex should be working across platfotms
        #    tested on FreeBSD, OpenBSD, NetBSD with success
        #    I prefer the starting with (^) rather than filtering throung all text
        # 2. /proc/mounts is not available on all *BSDs, needs revision
        proc_mounts=$( grep "^${device_name} " /proc/mounts )
        [ -n "${proc_mounts}" ]

[ "${#}" -ne 1 ] && { echo "Invalid number of arguments."; exit 1; }
readonly raw_input_string=${1}

is_empty_string "${raw_input_string}" && { echo "The given argument is empty."; exit 1; }
readonly device_string=$( sanitize_device_string "${raw_input_string}" )

is_empty_string "${device_string}" && { echo "The given argument is not a device path."; exit 1; }
! is_block_device "${device_string}" && { echo "The given argument is not a block device."; exit 1; }
readonly block_device=${device_string}

if is_device_uuid_identified "${block_device}"
    readonly device_name=$( translate_uuid_to_device_name "${block_device}" )
    readonly device_name=${block_device}

if is_device_mounted "${device_name}"
    echo "The device: ${block_device} IS mounted."
    echo "The device: ${block_device} IS NOT mounted."

Nice work. I approve of set -eu, and the script pleases Shellcheck.

Here's the things I'd consider changing.

I think error messages should go to the error stream. Example:

[ "${#}" -ne 1 ] && { echo "Invalid number of arguments." >&2; exit 1; }
#                                                         ^^^ here

Instead of using the negation operator, I'd replace the form ! test && error with plain test || error like this:

is_block_device "${device_string}" || { echo "The given argument is not a block device." >&2; exit 1; }

The script doesn't work when I use other links to block devices, such as those in /dev/disk/by-label. I'd fix that by abandoning the /by-uuid/ test, and instead following symlinks until a real file or dangling link is found:

resolve_symlink() {
    while [ -h "$f" ]
    do f=$(readlink -f -n "$f")
    printf '%s' "$f"
is_empty_string "${device_string}" && { echo "The given argument is not a device path." >&2; exit 1; }
is_block_device "${device_string}" || { echo "The given argument is not a block device." >&2; exit 1; }

readonly device_name=$(resolve_symlink "$device_string")

if is_device_mounted "$device_name"

Why does is_device_mounted ignore its argument and use $device_name instead?

Minor issue: we assume that the block device name contains no regex metacharacters here:

grep "^${device_name} "

That's probably a fair assumption on a non-weird Linux system; I normally use Awk for robust versions such tests ($1 = $device_name, with a suitable -v option) but I don't know how well that meets your portability goals.

If using grep (without the non-standard -q option), then it's usual to discard the output, and use grep's exit status directly, rather than capturing the output and testing it's non-empty.

Minor/style: I'm not a big fan of using braces for every variable expansion. I prefer to reserve them for when they are really needed, and that seems to be the usual idiom.


Your Answer

By clicking “Post Your Answer”, you agree to our terms of service, privacy policy and cookie policy

Not the answer you're looking for? Browse other questions tagged or ask your own question.