Mounting multiple partitions on a disk

Question

Is it possible to improve the code of this bash script? ... or should I just use another language? (If so, which one would fit the best the situation, according to you?)

Here is the code (simplified, to emphasis the problem):

#!/bin/bash
# mountDDE mount the partitions of the disk b.
# If arguments are passed, it will mount the specific partitions, a, b or c.

if [ -v "$1" ] # If no argument is passed, act on all partitions.
then a=t
    b=t
    c=t
fi

while [ -n "$1" ] ; do
    if [ $1="a" ] # We test the argument(s), to know which partition has to be mounted.
    then a=t
    elif [ $1="b"] # I hate redundunces
    then b=t
    elif [ $1="c"] # Again...
    then c=t
    fi
    shift
done

[ $a="t" ] && sudo mount /dev/sba /media/ba # I'll actually use uuids.
[ $b="t" ] && sudo mount /dev/sbb /media/bb # Not direct paths, like here.
[ $c="t" ] && sudo mount /dev/sbc /media/bc

To me, this code is redundant. Is there any more efficient/better way to do it ? Which language should I use for this task?

Answer 1

Bugs

First of all, you have some bugs, all of this type:

[ $1="b"]

There are multiple problems here:

• syntax error: there must be a space before the closing ]
• syntax error: there must be spaces around the =
• buggy: if $1 contains spaces, the script will crash
• pointless: no need to quote "b"

This would be correct and better:

[ "$1" = b ]

But actually, [ ... ] is obsoleted in favor of [[ ... ]]. Today, it's recommend to use [[ ... ]], which is less troublesome to write, for example you don't need to quote $1, this is correct:

[[ $1 = b ]]

Simplify

The many if-elif in the middle would be simpler with a case statement:

while [[ $1 ]] ; do
    case "$1" in
    a) a=t ;;
    b) b=t ;;
    c) c=t ;;
    esac
    shift
done

But actually, instead of a while loop and shifting, it's more natural to iterate over the command line arguments with a for loop:

for drive; do
    case $drive in
    a) a=t ;;
    b) b=t ;;
    c) c=t ;;
    esac
done

Suggested implementation

This would be a more compact solution (with one caveat, see at the end):

#!/bin/bash

if [[ $# = 0 ]]; then
    set -- a b c
fi

for x; do
    case $x in
    [abc]) sudo mount /dev/sb$x /media/b$x ;;
    esac
done

Note that this is not exactly the same as your original: if you run with script.sh a a a it will try to mount /dev/sda 3 times. It would be possible to tweak it further to guard against such misuses, but I would draw the line here. It's unlikely that somebody would misuse the script that way, it won't be the end of the world, and the script in its current form is short and sweet, ain't it?

Answer 2

Since you're using bash (I assume a recent version of bash), you can use some advanced features, like associative arrays and indirect variables

#!/bin/bash

# If no argument is passed, act on all partitions.
if (( $# == 0 )); then
    a=t b=t c=t
else
    for arg; do
        if [[ $arg == a|b|c ]]; then
            declare $arg=t
        else
            echo "ignoring argument '$arg'"
        fi
    done
fi

declare -A uuids=( [a]=1234 [b]=3456 [c]=5678 )

for dest in a b c; do
    if [[ ${!dest} == t ]]; then
        sudo mount /dev/sb$dest "${uuids[$dest]}"
    fi
done

Note that the [ command acts differently when given different numbers of arguments. With just a single argument (such as [ $a="t" ]), you'll get a success exit status if the argument is not empty (and the string "$a=t" is not empty no matter the value of $a). You have to put spaces between all the operators and operands for [ and test and [[.

Answer 3

This looks more streamlined. I suppose you need to massage it a little for your purposes.

function do_mount()
{
    while [ -n "$1" ]; do
        sudo mount /dev/sb"$1" /media/b"$1"
    done
}

if [ -v "$@" ]; then
    do_mount a b c
else
    do_mount "$@"
fi

Answer 4

I replaced the -v with -z as -v doesn't seem to apply to $1. -z is true when string length is 0.

Also, why over-usage of setting variables? I read what you want as being in two states: you either provide a variable and want all partitions mounted or you provide arguments and want just those partitions mounted. I see that as a single if then else statement!

I list each mount individually, while this may seem redundant, compacting it even more results in difficult to read code blocks that work perfect in their original form, but nearly impossible to modify to your own usage (in this case, /dev/sdx -> UUID)

I don't mean to detract from what others wrote, but my reasoning is exampled in previous posts

[abc]) sudo mount /dev/sb$x /media/b$x

sudo mount /dev/sb"$1" /media/b"$1"

You specify wishing to use UUIDS, I personally wouldn't know how to modify the above to accommodate UUIDS.

However, scaled mine would not work well as it requires asking each argument 3 questions. For only 3 though, I believe it's fine.

UUID_A=''
UUID_B=''
UUID_C=''
if [ -z "$1" ] # If no argument is passed, act on all partitions.
    then
        sudo mount $UUID_A /media/ba
        sudo mount $UUID_B /media/bb
        sudo mount $UUID_C /media/bc
    else
        for i in $@;
            do
                [ "$i" ==  a ] && sudo mount $UUID_A /media/ba 
                [ "$i" ==  b ] && sudo mount $UUID_B /media/bb
                [ "$i" ==  c ] && sudo mount $UUID_C /media/bc
            done
fi

Answer 5

There is one more thing nobody has mentioned yet: Error handling

1. Say the user calls `script.sh a b d e a z'.
   Only a, b and c are valid arguments. The rest should complain. a is used twice. Wether to complain about that or ignore it is a matter of taste.

2. mount can fail. Say the user calls script.sh a b and mounting of a fails.
   Your script will continue to mount b. If that works the script returns success. The error for a gets lost.
   There are two ways to improve this. One is to add set -e at the begining. The script will then fail on the first command that fails. The other is to catch the return code of mount using $? or ||, record the failure in a variable and at the end exit with an error (e.g. the number of mounts that failed) if any mount failed.
   Actually there is a third option. If any mount fails you can undo the mounts that already succeeded and exit with an error. Then you have an all or nothing semantic.