On Sat, 9 May 2009, Rogério Brito wrote:
> Hi,
>
> I am triaging some bugs of usbmount to see if they are still
> reproducible (some still seem to be, but others do not---or, at least, I
> can't).
>
> Could you please let me know if you still see the problem that you've
> reported with the latest package (version 0.0.17) from unstable (soon to
> be migrated to testing)?
>
> Please, if you find any new bugs, don't hesitate to file another one
> against usbmount.
>
> If you submitted a patch, can you see if it is still needed? I would
> like to kill as many bugs as possible with an upcoming version.
Hi Rogério,
This is the first time I see the above. I picked up the mbox from here:
http://bugs.debian.org/cgi-bin/bugreport.cgi?msg=30;mbox=yes;bug=502352
so I can answer.
I've made some modifications (enhancements I would like to believe :) to
the scripts and conf. Robustified a bit the code handling UUID mounts
too. Rediffed against 0.0.18 and attached. You might find some stuff in
there you want to use.
Cheers,
--
Cristian--- etc/usbmount/usbmount.conf.orig 2009-10-21 06:20:58.000000000 +0200
+++ etc/usbmount/usbmount.conf 2009-10-25 11:16:27.000000000 +0100
@@ -42,6 +42,10 @@ MOUNTOPTIONS="sync,noexec,nodev,noatime,
FS_MOUNTOPTIONS=""
-# If set to "yes", more information will be logged via the syslog
-# facility.
-VERBOSE="no"
+# If set to "yes", more information will be logged via the syslog facility.
+VERBOSE=no
+
+# used in /usr/share/usbmount/usbmount
+# /etc/usbmount/mount.d/00_create_model_symlink
+# /etc/usbmount/umount.d/00_remove_model_symlink
+SYMLINK_MODEL_DIR=/var/run/usbmount
--- etc/usbmount/mount.d/00_create_model_symlink.orig 2008-10-19 10:46:37.000000000 +0200
+++ etc/usbmount/mount.d/00_create_model_symlink 2009-10-25 11:21:19.000000000 +0100
@@ -13,28 +13,46 @@
#
set -e
+self=${0##*/}
+log="logger -p user.warning -t $self[$$]"
+
+[ ! -r /etc/usbmount/usbmount.conf ] || . /etc/usbmount/usbmount.conf
+
# Replace spaces with underscores, remove special characters in vendor
# and model name.
-UM_VENDOR=`echo "$UM_VENDOR" | sed 's/ /_/g; s/[^0123456789abcdefghijklmnopqrstuvwxyzABCDEFGHIJKLMNOPQRSTUVWXYZ._-]//g'`
-UM_MODEL=`echo "$UM_MODEL" | sed 's/ /_/g; s/[^0123456789abcdefghijklmnopqrstuvwxyzABCDEFGHIJKLMNOPQRSTUVWXYZ._-]//g'`
+UM_VENDOR=`echo "$UM_VENDOR" | sed -e 's/[[:blank:]]\+/_/g; s/[^[:alnum:]._-]//g'`
+UM_MODEL=`echo "$UM_MODEL" | sed -e 's/[[:blank:]]\+/_/g; s/[^[:alnum:]._-]//g'`
# Exit if both vendor and model name are empty.
-test -n "$UM_VENDOR" || test -n "$UM_MODEL" || exit 0
+[ "$UM_VENDOR" ] || [ "$UM_MODEL" ] || {
+ case "$VERBOSE" in
+ [Yy]*)
+ $log "empty vendor and model name"
+ ;;
+ esac
+ exit 0
+}
# Build symlink name.
-if test -n "$UM_VENDOR" && test -n "$UM_MODEL"; then
+if [ "$UM_VENDOR" ] && [ "$UM_MODEL" ]; then
name="${UM_VENDOR}_$UM_MODEL"
else
name="$UM_VENDOR$UM_MODEL"
fi
# Append partition number, if any, to the symlink name.
-partition=`echo "$UM_DEVICE" | sed 's/^.*[^0123456789]\([0123456789]*\)/\1/'`
-if test -n "$partition"; then
- name="${name}_$partition"
-fi
+partition=`echo "$UM_DEVICE" | sed -e 's/^.*[^0-9]\([0-9]*\)/\1/'` || :
+[ -z "$partition" ] || name="${name}_$partition"
# If the symlink does not yet exist, create it.
-test -e "/var/run/usbmount/$name" || ln -sf "$UM_MOUNTPOINT" "/var/run/usbmount/$name"
+# If the symlink points to the wrong thing, recreate it.
+syml=$SYMLINK_MODEL_DIR/$name
+if [ -e "$syml" ]; then
+ [ "$(readlink "$syml" || :)" != "$UM_MOUNTPOINT" ] || exit 0
+ $log "symlink '$syml' and mount point '$UM_MOUNTPOINT' mismatch;" \
+ "removing symlink"
+ rm -f "$syml"
+fi
+ln -sf "$UM_MOUNTPOINT" "$syml"
exit 0
--- etc/usbmount/umount.d/00_remove_model_symlink.orig 2008-10-19 10:46:37.000000000 +0200
+++ etc/usbmount/umount.d/00_remove_model_symlink 2009-10-25 11:22:55.000000000 +0100
@@ -1,5 +1,5 @@
#!/bin/sh
-# This script removes the model name symlink in /var/run/usbmount.
+# This script removes the model name symlink in $SYMLINK_MODEL_DIR.
# Copyright (C) 2005 Martin Dickopp
#
# This file is free software; the copyright holder gives unlimited
@@ -13,9 +13,16 @@
#
set -e
-ls /var/run/usbmount | while read name; do
- if test "`readlink \"/var/run/usbmount/$name\" || :`" = "$UM_MOUNTPOINT"; then
- rm -f "/var/run/usbmount/$name"
+[ ! -r /etc/usbmount/usbmount.conf ] || . /etc/usbmount/usbmount.conf
+
+cd $SYMLINK_MODEL_DIR
+IFS='
+'
+for name in *; do
+ [ -L "$name" ] || continue
+ syml=$SYMLINK_MODEL_DIR/$name
+ if [ "$(readlink "$syml" || :)" = "$UM_MOUNTPOINT" ]; then
+ rm -f "$syml"
break
fi
done
--- usr/share/usbmount/usbmount.orig 2009-10-21 06:20:58.000000000 +0200
+++ usr/share/usbmount/usbmount 2009-10-25 12:59:25.000000000 +0100
@@ -14,26 +14,39 @@
# PARTICULAR PURPOSE.
#
set -e
-exec > /dev/null 2>&1
+
+self=${0##*/}
+TAG=$self[$$]
+
+#//#exec > /dev/null 2>&1
+set -x
+exec > /tmp/$self.$$.log 2>&1
+
+is_verbose() {
+ case $VERBOSE in
+ [Yy]*)
+ return 0
+ ;;
+ esac
+ return 1
+}
######################################################################
-# Auxiliary functions
+# Auxiliary functions
# Log a string via the syslog facility.
-log()
-{
- if test $1 != debug || expr "$VERBOSE" : "[yY]" > /dev/null; then
- logger -p user.$1 -t "usbmount[$$]" -- "$2"
+log() {
+ if [ "$1" != debug ] || is_verbose; then
+ logger -p user.$1 -t $TAG -- "$*"
fi
}
# Test if the first parameter is in the list given by the second
# parameter.
-in_list()
-{
+in_list() {
for v in $2; do
- test "$1" != "$v" || return 0
+ [ "$1" != "$v" ] || return 0
done
return 1
}
@@ -42,56 +55,66 @@ in_list()
######################################################################
# Main program
+#//# No point continuing if /sbin/blkid is not available.
+blkid=$(which blkid 2>/dev/null) || {
+ log err "cannot locate blkid"
+ exit 1
+}
+
# Default values for configuration variables.
ENABLED=1
-MOUNTPOINTS=""
-FILESYSTEMS=""
-MOUNTOPTIONS=""
-FS_MOUNTOPTIONS=""
-VERBOSE="no"
+#//# Quoting these values adds nothing useful.
+MOUNTPOINTS=
+FILESYSTEMS=
+MOUNTOPTIONS=
+FS_MOUNTOPTIONS=
+VERBOSE=no
+CONF=/etc/usbmount/usbmount.conf
+FSTAB=/etc/fstab
# Read configuration file.
-if [ -r /etc/usbmount/usbmount.conf ]; then
- . /etc/usbmount/usbmount.conf
- log debug "loaded usbmount configurations"
+if [ -r $CONF ]; then
+ . $CONF
+ log debug "configuration loaded"
fi
# Check if usbmount should be run.
-if [ "${ENABLED:-1}" -eq "0" ]; then
- log info "usbmount is disabled, see /etc/usbmount/usbmount.conf"
+#//# Arithmetic, not string evaluation.
+if [ ${ENABLED:-1} -eq 0 ]; then
+ log info "usbmount is disabled, see $CONF"
exit 0
fi
-# Test if blkid is executable.
-if [ ! -x /sbin/blkid ]; then
- log err "cannot execute /sbin/blkid"
- exit 1
-fi
-
-# Per Policy 9.3.2, directories under /var/run/usbmount have to be checked
+# Per Policy 9.3.2, directories under /var/run have to be checked
# after every reboot.
-if [ ! -e /var/run/usbmount ]; then
- mkdir -p /var/run/usbmount
- log debug "creating /var/run/usbmount directory"
-fi
+#//# /var/run/usbmount is used in a lot of places; use a $CONF variable instead.
+[ -d "$SYMLINK_MODEL_DIR" ] || mkdir -p "$SYMLINK_MODEL_DIR" || {
+ log err "cannot create the $SYMLINK_MODEL_DIR directory"
+ exit 1
+}
umask 022
-if [ "$1" = "add" ]; then
-
+#//# Useless quotes removed.
+if [ "$1" = add ]; then
# Acquire lock.
- log debug "trying to acquire lock /var/run/usbmount/.mount.lock"
- lockfile-create --retry 3 /var/run/usbmount/.mount || \
- { log err "cannot acquire lock /var/run/usbmount/.mount.lock"; exit 1; }
- trap '( lockfile-remove /var/run/usbmount/.mount )' 0
- log debug "acquired lock /var/run/usbmount/.mount.lock"
+ log debug "trying to acquire lock $SYMLINK_MODEL_DIR/.mount.lock"
+ lockfile-create --retry 3 "$SYMLINK_MODEL_DIR/.mount" || {
+ log err "cannot acquire lock $SYMLINK_MODEL_DIR/.mount.lock"
+ exit 1
+ }
+ #//# The trap does not need to be run in a subshell?
+ trap 'lockfile-remove "$SYMLINK_MODEL_DIR/.mount"' EXIT
+ log debug "acquired lock $SYMLINK_MODEL_DIR/.mount.lock"
# Try to read from the device. Some devices need a few seconds
# initialization time before they can be accessed. Give up after
# 20 seconds. Thanks to Peter Stelmachovic for his help with
# debugging this.
log debug "testing whether $DEVNAME is readable"
- read_success=no; T=0; RETRIES=20
+ read_success=no
+ T=0
+ RETRIES=20
while [ $T -lt $RETRIES ]; do
if dd if="$DEVNAME" of=/dev/null bs=512 count=1; then
read_success=yes
@@ -102,35 +125,39 @@ if [ "$1" = "add" ]; then
sleep 1
done
- if [ "$read_success" != "yes" ]; then
+ #//# Removed useless quotes.
+ if [ $read_success != yes ]; then
log err "could not read from $DEVNAME for $RETRIES seconds; aborting"
exit 1
fi
- UUID=$(/sbin/blkid "$DEVNAME" | sed -e 's/.*UUID="\([^"]*\)".*/\1/g; s/\s*//g;')
- # Test if the device has an /etc/fstab entry. In that case, we will
+ #//# Use regex class [:blank:].
+ UUID=$($blkid "$DEVNAME" |
+ sed -e 's/.*UUID="\([^"]*\)".*/\1/g; s/[[:blank:]]\+//g')
+ # Test if the device has an fstab entry. In that case, we will
# mount it using the regular mount command.
- if grep -q "^[ ]*$DEVNAME" /etc/fstab; then
- log debug "$DEVNAME has an /dev/fstab entry, using that"
+ if egrep -wq "^[[:blank:]]*$DEVNAME" $FSTAB; then
+ log debug "$DEVNAME has an /dev/fstab entry, using that"
# Mount the filesystem.
log info "executing command: mount $DEVNAME"
mount "$DEVNAME"
- # Test if the device has an /etc/fstab entry with its UUID, in this
- # case we will mount it with a mount command on the mount point
- elif grep -q $UUID /etc/fstab; then
- log debug "$DEVNAME has an /etc/fstab entry, with UUID $UUID, using that"
-
- MOUNT_POINT="`grep $UUID /etc/fstab|awk '{print $2}'`"
- log info "executing command: mount $MOUNT_POINT"
+ # Test if the device has an UUID fstab entry and mount using that.
+ elif [ "$UUID" ] && egrep -q "^[[:blank:]]*UUID=$UUID[[:blank:]]" $FSTAB; then
+ log debug "$DEVNAME has an $FSTAB entry, with UUID $UUID, using that"
+
+ MOUNT_POINT=$(sed -rne "s|^[[:blank:]]*UUID=$UUID[[:blank:]]+([^[:blank:]]+)[[:blank:]]+.*$|\1|p" $FSTAB)
+ #//# Are MOUNT_POINT and DEVNAME identical?
+ log info "executing command: mount $MOUNT_POINT"
mount "$DEVNAME"
# Test if the device contains a filesystem.
- elif /sbin/blkid -p -o udev "$DEVNAME" | egrep -q '^ID_FS_USAGE=(filesystem|disklabel)$'; then
+ elif $blkid -p -o udev "$DEVNAME" | egrep -q '^ID_FS_USAGE=(filesystem|disklabel)$'; then
log debug "$DEVNAME contains a filesystem or disklabel"
- fstype=$(/sbin/blkid -s TYPE -o value "$DEVNAME" | sed -e 's/\s*//g;')
+ #//# Is removing whitespace here the right thing to do?
+ fstype=$($blkid -s TYPE -o value "$DEVNAME" | sed -e 's/[[:blank:]]//g')
log debug "$DEVNAME contains filesystem type $fstype"
# Test if the filesystem type is in the list of filesystem
@@ -139,29 +166,32 @@ if [ "$1" = "add" ]; then
# Search an available mountpoint.
for v in $MOUNTPOINTS; do
- if test -d "$v" \
- && ! grep -q "^[^ ][^ ]* *$v " /proc/mounts; then
- mountpoint="$v"
+ if [ -d "$v" ] &&
+ ! egrep -q "^[^[:blank:]]+[[:blank:]]+$v[[:blank:]]+" /proc/mounts; then
+ mountpoint=$v
log debug "mountpoint $mountpoint is available for $DEVNAME"
break
fi
done
- if test -n "$mountpoint"; then
+ if [ "$mountpoint" ]; then
# Determine mount options.
options=
for v in $FS_MOUNTOPTIONS; do
- if expr "$v" : "-fstype=$fstype,."; then
- options="`echo \"$v\" | sed 's/^[^,]*,//'`"
+ #//# Optimized.
+ if expr "$v" : "-fstype=$fstype,." >/dev/null; then
+ options=${v#*,}
break
fi
done
- if test -n "$MOUNTOPTIONS"; then
- options="$MOUNTOPTIONS${options:+,$options}"
- fi
+ #//# Shortened.
+ [ -z "$MOUNTOPTIONS" ] ||
+ options="$MOUNTOPTIONS${options:+,$options}"
# Mount the filesystem.
- log info "executing command: mount -t$fstype ${options:+-o$options} $DEVNAME $mountpoint"
- mount "-t$fstype" "${options:+-o$options}" "$DEVNAME" "$mountpoint"
+ #//# Optimized.
+ cmd="mount -t$fstype ${options:+-o$options} $DEVNAME $mountpoint"
+ log info "executing command: $cmd"
+ $cmd
# Determine vendor and model.
vendor=
@@ -174,7 +204,8 @@ if [ "$1" = "add" ]; then
elif [ -r "/sys$DEVPATH/../device/../manufacturer" ]; then
vendor="`cat \"/sys$DEVPATH/../device/../manufacturer\"`"
fi
- vendor="`echo \"$vendor\" | sed 's/^ *//; s/ *$//'`"
+ vendor=$(echo "$vendor" |
+ sed -e 's/^[[:blank:]]\+//; s/[[:blank:]]\+$//')
model=
if [ -r "/sys$DEVPATH/device/model" ]; then
@@ -186,9 +217,11 @@ if [ "$1" = "add" ]; then
elif [ -r "/sys$DEVPATH/../device/../product" ]; then
model="`cat \"/sys$DEVPATH/../device/../product\"`"
fi
- model="`echo \"$model\" | sed 's/^ *//; s/ *$//'`"
+ model=$(echo "$model" |
+ sed -re 's/^[[:blank:]]\+//; s/[[:blank:]]\+$//')
# Run hook scripts; ignore errors.
+ #//# Most quoting here is superfluous.
export UM_DEVICE="$DEVNAME"
export UM_MOUNTPOINT="$mountpoint"
export UM_FILESYSTEM="$fstype"
@@ -206,17 +239,15 @@ if [ "$1" = "add" ]; then
else
log debug "$DEVNAME does not contain a filesystem or disklabel"
fi
-
-elif test "$1" = remove; then
-
+elif [ "$1" = remove ]; then
# A block or partition device has been removed.
# Test if it is mounted.
while read device mountpoint fstype remainder; do
if [ "$DEVNAME" = "$device" ]; then
# If the mountpoint and filesystem type are maintained by
# this script, unmount the filesystem.
- if in_list "$mountpoint" "$MOUNTPOINTS" \
- && in_list "$fstype" "$FILESYSTEMS"; then
+ if in_list "$mountpoint" "$MOUNTPOINTS" &&
+ in_list "$fstype" "$FILESYSTEMS"; then
log info "executing command: umount -l $mountpoint"
umount -l "$mountpoint"
@@ -224,13 +255,14 @@ elif test "$1" = remove; then
export UM_DEVICE="$DEVNAME"
export UM_MOUNTPOINT="$mountpoint"
export UM_FILESYSTEM="$fstype"
- log info "executing command: run-parts /etc/usbmount/umount.d"
- run-parts /etc/usbmount/umount.d || :
+ #//# Optimized.
+ cmd="run-parts /etc/usbmount/umount.d"
+ log info "executing command: $cmd"
+ $cmd || :
fi
break
fi
done < /proc/mounts
-
fi
exit 0
