patternshellMinor
Mount different remote systems
Viewed 0 times
remotedifferentsystemsmount
Problem
I have a shell script to mount different remote systems. While it does work, the area I struggle with (not being a programmer) is the logic of the script: it feels quite stilted and there seems to be some unnecessary duplication.
I would appreciate any comments as to how I could improve it - both in terms of the logic, but also any other tips about style or approach.
I have obscured the relevant ports and hostnames.
I would appreciate any comments as to how I could improve it - both in terms of the logic, but also any other tips about style or approach.
#!/bin/sh
# mount boxes by SSHFS
usage () {
cat <<EOF
sshmnt -[c,s,u,h]
-c Box1
-s Box2
-u unmount
-h print this message
EOF
}
mnt(){
sshfs jason@"$host":/home/jason /media/"$dir" \
-C -p "$port" \
-o reconnect,IdentityFile=/home/jason/.ssh/id_rsa \
&& ls /media/"$dir"
}
umnt(){
fusermount -u /media/"$dir"
}
# unmounting
if [ "$1" = "-u" ]; then
box=$(mount | grep Box1)
if [ -n "$box" ]; then
dir=Box1
else
dir=Box2
fi
umnt && exit
fi
# check if on LAN
lan="$(ip addr | grep .102/)"
if [ -n "$lan" ]; then
case "$1" in
-c) int=100
;;
-s) int=200
;;
esac
host="192.168.1.$int"
else
host="XXX.XXX.XXX.XXX"
fi
# box specifics
case "$1" in
-c) dir=Box1 port="XXXX"
mnt
;;
-s) dir=Box2 port="XXXX"
mnt
;;
*) usage && exit
;;
esacI have obscured the relevant ports and hostnames.
Solution
Your functions shouldn't depend on variables but use specified
parameters. You always specified variables before calling your function instead of just specifying them as parameters, e.g:
but you can easily write it as
I would also only parse your command line arguments in one place and create
a dedicated function for determine the appropriate ip address for your hosts.
I would also remove the ls call from your
Another suggestion is to get rid of all your hard coded paths and texts like,
and just specify the user for your hosts in
parameters. You always specified variables before calling your function instead of just specifying them as parameters, e.g:
DIR=Box1
Host=192.168.1.100
mntbut you can easily write it as
mnt Box1 192.168.1.100I would also only parse your command line arguments in one place and create
a dedicated function for determine the appropriate ip address for your hosts.
I would also remove the ls call from your
mnt function as mnt doesn't do what the name suggests. But this is a rather minor issue and I didn't change it in my code.#!/bin/sh
# mount boxes by SSHFS
usage () {
cat &2
return 1
fi
host="$1"
dir="$2"
port="$3"
sshfs jason@"$host":/home/jason /media/"$dir" \
-C -p "$port" \
-o reconnect,IdentityFile=/home/jason/.ssh/id_rsa \
&& ls /media/"$dir"
}
umnt(){
dir="$1"
fusermount -u /media/"$dir"
}
get_host(){
# check if on LAN
lan="$(ip addr | grep .102/)"
if [ -z "$lan" ] ; then
echo XXX.XXX.XXX.XXX
return 0
fi
case "$1" in
Box1) echo 192.168.1.100 ;;
Box2) echo 192.168.1.200 ;;
*) echo "Unknown Parameter" >&2 && return 1 ;;
esac
}
case "$1" in
-c) mnt $(get_host Box1) Box1 port ;;
-s) mnt $(get_host Box2) Box2 port ;;
-u)
mount | grep Box1 && umnt Box1
mount | grep Box2 && umnt Box2
;;
*) usage && exit 1;;
esacAnother suggestion is to get rid of all your hard coded paths and texts like,
/media/, /home/jason/ and jason and replace them with variables, e.g:MOUNT_PREFIX=/media/
SSH_KEY=/home/jason/.ssh/id_rsa
MOUNT_SOURCE=/home/jasonand just specify the user for your hosts in
~/.ssh/configCode Snippets
DIR=Box1
Host=192.168.1.100
mntmnt Box1 192.168.1.100#!/bin/sh
# mount boxes by SSHFS
usage () {
cat <<EOF
sshmnt -[c,s,u,h]
-c Box1
-s Box2
-u unmount
-h print this message
EOF
}
mnt(){
if [ $# -ne 3 ] ; then
echo "Wrong parameters for mnt - host dir port" >&2
return 1
fi
host="$1"
dir="$2"
port="$3"
sshfs jason@"$host":/home/jason /media/"$dir" \
-C -p "$port" \
-o reconnect,IdentityFile=/home/jason/.ssh/id_rsa \
&& ls /media/"$dir"
}
umnt(){
dir="$1"
fusermount -u /media/"$dir"
}
get_host(){
# check if on LAN
lan="$(ip addr | grep .102/)"
if [ -z "$lan" ] ; then
echo XXX.XXX.XXX.XXX
return 0
fi
case "$1" in
Box1) echo 192.168.1.100 ;;
Box2) echo 192.168.1.200 ;;
*) echo "Unknown Parameter" >&2 && return 1 ;;
esac
}
case "$1" in
-c) mnt $(get_host Box1) Box1 port ;;
-s) mnt $(get_host Box2) Box2 port ;;
-u)
mount | grep Box1 && umnt Box1
mount | grep Box2 && umnt Box2
;;
*) usage && exit 1;;
esacMOUNT_PREFIX=/media/
SSH_KEY=/home/jason/.ssh/id_rsa
MOUNT_SOURCE=/home/jasonContext
StackExchange Code Review Q#14424, answer score: 8
Revisions (0)
No revisions yet.