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?

• 200_success: What you may and may not do after receiving answers. I've rolled back Rev 4 → 1.

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?

• 200_success: [[ may be more featureful than [, but [ is not deprecated.
• loxaxs: [[ $# -gt 0 ]] won't work, we want [[ $# -le 0 ]] isn't it ?
• janos: Thanks loxaxs, I overlooked that. @200_success the answer you linked has no proof, and it's not recent. I don't have the proof either, I'll have to find the link again. I learned this on Unix.se and I was surprised too. Or maybe I remember wrong. I'll get back to you on this tomorrow.
• h.j.k.: mywiki.wooledge.org/BashFAQ/031 It boils down to how strict a POSIX compatibility you want, or whether you are code-golfing (saving 2 chars) or not... :)
• janos: @200_success you're right, it's not "deprecated", it's "obsoleted", according to this article: wiki.bash-hackers.org/scripting/obsolete

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

• glenn jackman: You may get an error with that usage of -v: [ -v a b c ] gives bash: [: too many arguments
• glenn jackman: Infinite loop: You need a shift in your function.
• loxaxs: glenn jackman: Actually, it's me who made that error first, and silently removed it (edit) after vnp answered.

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.