* Some questions
@ 2003-08-19 15:53 Joerg Sommer
0 siblings, 0 replies; only message in thread
From: Joerg Sommer @ 2003-08-19 15:53 UTC (permalink / raw)
To: linux-hotplug
Hi,
I've downloaded hotplug-2003_08_05 and have some questions about the code:
* Why not remove the bashism?
sbin/hotplug:
* Why not include the test for -x in the if expression?
if [ -f $I -a -x $I ]; then
$I $1 ;
fi
* Why returns this script with exit code 1? Doesn't this stand for an
error?
#v+
diff -ur hotplug-2003_08_05.orig/sbin/hotplug hotplug-2003_08_05/sbin/hotplug
--- hotplug-2003_08_05.orig/sbin/hotplug 2003-06-28 02:13:10.000000000 +0
+++ hotplug-2003_08_05/sbin/hotplug 2003-08-19 16:21:38.000000000 +0200
@@ -28,8 +28,8 @@
DIR="/etc/hotplug.d"
for I in "${DIR}/$1/"*.hotplug "${DIR}/"default/*.hotplug ; do
- if [ -f $I ]; then
- test -x $I && $I $1 ;
+ if [ -f $I -a -x $I ]; then
+ $I $1
fi
done
#v-
etc/hotplug.d/default/default.hotplug:
* l.46: Why use sed to strip the tailing .agent? This causes a pipe
and another command, even though, this could be done by basename.
* l.67: Why do you debug handling by hand? There is a function
debug_mesg, so this shouldn't be necessary.
* If default.hotplug is called in the right way, you check, if the agent
is executable. If not, it fails. So you can't name nonexecutable agents
in l.53.
* I think a $(set -e) is essential, because a cd to /etc/hotplug could
fail.
* To source hotplug.functions a path must be named, otherwise
hotplug.functions is searched in the directories of $PATH, which should
fail. See Posix 1003.2 3.14.4.
#v+
diff -ur hotplug-2003_08_05.orig/etc/hotplug.d/default/default.hotplug hotplug-2003_08_05/etc/hotplug.d/default/default.hotplug
--- hotplug-2003_08_05.orig/etc/hotplug.d/default/default.hotplug 2003-05-02 07:01:45.000000000 +0200
+++ hotplug-2003_08_05/etc/hotplug.d/default/default.hotplug 2003-08-19 16:26:07.000000000 +0200
@@ -23,10 +23,12 @@
# $Id: default.hotplug,v 1.1 2003/05/02 05:01:45 kroah Exp $
#
-cd /etc/hotplug
-. hotplug.functions
+set -e
+
+# export DEBUG=yes
-# DEBUG=yes export DEBUG
+cd /etc/hotplug
+. ./hotplug.functions
debug_mesg "arguments ($*) env (`env`)"
@@ -38,19 +40,16 @@
# through argv.
#
if [ $# -lt 1 -o "$1" = "help" -o "$1" = "--help" ]; then
- if [ -t ]; then
+ if [ -t 1 ]; then
echo "Usage: $0 AgentName [AgentArguments]"
- AGENTS=""
- for AGENT in /etc/hotplug/*.agent ; do
- TYPE=`basename $AGENT | sed s/.agent//`
+ printf "AgentName values on this system:"
+ for AGENT in $HOTPLUG_DIR/*.agent ; do
if [ -x $AGENT ]; then
- AGENTS="$AGENTS $TYPE"
- else
- AGENTS="$AGENTS ($TYPE)"
+ printf " `basename $AGENT .agent`"
fi
done
- echo "AgentName values on this system: $AGENTS"
+ echo
else
mesg "illegal usage $*"
fi
@@ -61,12 +60,10 @@
# Delegate event handling:
# /sbin/hotplug FOO ..args.. ==> /etc/hotplug/FOO.agent ..args..
#
-AGENT=/etc/hotplug/$1.agent
+AGENT=$HOTPLUG_DIR/$1.agent
if [ -x $AGENT ]; then
shift
- if [ "$DEBUG" != "" ]; then
- mesg "invoke $AGENT ($@)"
- fi
+ debug_mesg "invoke $AGENT ($@)"
exec $AGENT "$@"
mesg "couldn't exec $AGENT"
exit 1
#v-
etc/hotplug/hotplug.functions:
* Posix logger doesn't know the option -t. If you depend on BSD logger,
it may be possible to specify the priority of the messages. In this
case, I would propose to add a function error_mesg(), hat logs on
stderr or use level debug for logger. As a whole, I would propose in
this case, to log to facility daemon.
#v+
diff -ur hotplug-2003_08_05.orig/etc/hotplug/hotplug.functions hotplug-2003_08_05/etc/hotplug/hotplug.functions
--- hotplug-2003_08_05.orig/etc/hotplug/hotplug.functions 2003-06-28 02:13:10.000000000 +0200
+++ hotplug-2003_08_05/etc/hotplug/hotplug.functions 2003-08-19 17:36:55.000000000 +0200
@@ -10,7 +10,7 @@
#
#
-# DEBUG=yes; export DEBUG
+# export DEBUG=yes
PATH=/bin:/sbin:/usr/sbin:/usr/bin
KERNEL=`uname -r`
@@ -22,31 +22,31 @@
. /etc/sysconfig/hotplug
fi
-if [ -x /usr/bin/logger ]; then
- LOGGER=/usr/bin/logger
-elif [ -x /bin/logger ]; then
- LOGGER=/bin/logger
-else
- unset LOGGER
-fi
#
# for diagnostics
#
-if [ -t -o -z "$LOGGER" ]; then
+if [ -t 1 -o ! \( -x /usr/bin/logger -o -x /bin/logger \) ]; then
mesg () {
echo "$@"
}
+ if [ "$DEBUG" = yes ]; then
+ debug_mesg () {
+ echo "$@"
+ }
+ else
+ debug_mesg () {
+ :
+ }
+ fi
else
mesg () {
- $LOGGER -t $(basename $0)"[$$]" "$@"
+ logger $(basename $0)"[$$] $@"
+ }
+ debug_mesg () {
+ logger $(basename $0)"[$$] $@"
}
fi
-debug_mesg () {
- test "$DEBUG" = "" -o "$DEBUG" = no && return
- mesg "$@"
-}
-
#
# The modules.*map parsing uses BASH ("declare -i") and some version
# of AWK, typically /bin/gawk. Most GNU/Linux distros have these,
@@ -80,23 +80,18 @@
#
load_drivers ()
{
- local LOADED TYPE FILENAME DESCRIPTION LISTER
- DRIVERS=""
-
# make this routine more readable
TYPE=$1
FILENAME=$2
DESCRIPTION=$3
+ DRIVERS=""
# should we use usbmodules, pcimodules? not on 2.5+, because sysfs
# ought to expose the data we need to find all candidate drivers.
# (on 2.5.48 it does for usb; but maybe not yet for pci.)
case "$KERNEL" in
- 2.2*|2.3*|2.4*) LISTER=`type -p ${TYPE}modules` ;;
- *) LISTER="" ;;
- esac
-
- if [ "$LISTER" != "" ]; then
+ 2.2*|2.3*|2.4*)
+ LISTER=`type -p ${TYPE}modules`
# lister programs MIGHT be preferable to parsing from shell scripts:
# - usbmodules used for (a) multi-interface devices, (b) coldplug
# - pcimodules used only for coldplug
@@ -106,27 +101,26 @@
# only works if we have usbfs
# ... reads more descriptors than are passed in env
# ... doesn't handle comment syntax either
- if [ "$DEVICE" = "" -o ! -f "$DEVICE" ]; then
- LISTER=
- else
+ if [ -f "$DEVICE" ]; then
DRIVERS=`$LISTER --mapfile $FILENAME --device $DEVICE`
- fi ;;
-
+ fi
+ ;;
pci)
debug_mesg "pcimodules is scanning more than $PCI_SLOT ..."
DRIVERS=`$LISTER`
;;
esac
- fi
+ ;;
+ esac
# try parsing by shell scripts if no luck yet
- if [ "$DRIVERS" = "" ]; then
+ if [ -z "$DRIVERS" ]; then
${TYPE}_map_modules < $FILENAME
fi
# FIXME remove dups and blacklisted modules from $DRIVERS here
- if [ "$DRIVERS" = "" ]; then
+ if [ -z "$DRIVERS" ]; then
return
fi
@@ -139,9 +133,8 @@
do
# maybe driver modules need loading
LOADED=false
- if ! lsmod | grep -q "^$MODULE " > /dev/null 2>&1; then
- if grep -q "^$MODULE\$" $HOTPLUG_DIR/blacklist \
- >/dev/null 2>&1; then
+ if ! lsmod | grep -q "^$MODULE "; then
+ if grep -q "^$MODULE\$" $HOTPLUG_DIR/blacklist; then
debug_mesg "... blacklisted module: $MODULE"
continue
fi
@@ -167,8 +160,7 @@
# giving kernel code another chance.
if [ -x $HOTPLUG_DIR/$TYPE/$MODULE ]; then
debug_mesg Module setup $MODULE for $DESCRIPTION
- $HOTPLUG_DIR/$TYPE/$MODULE
- LOADED=true
+ $HOTPLUG_DIR/$TYPE/$MODULE && LOADED=true
fi
if [ $LOADED = false ]; then
#v-
Bye, Joerg.
--
»Perl - the only language that looks the same
before and after RSA encryption.« -- Keith Bostic
-------------------------------------------------------
This SF.net email is sponsored by Dice.com.
Did you know that Dice has over 25,000 tech jobs available today? From
careers in IT to Engineering to Tech Sales, Dice has tech jobs from the
best hiring companies. http://www.dice.com/index.epl?rel_code\x104
_______________________________________________
Linux-hotplug-devel mailing list http://linux-hotplug.sourceforge.net
Linux-hotplug-devel@lists.sourceforge.net
https://lists.sourceforge.net/lists/listinfo/linux-hotplug-devel
^ permalink raw reply [flat|nested] only message in thread
only message in thread, other threads:[~2003-08-19 15:53 UTC | newest]
Thread overview: (only message) (download: mbox.gz follow: Atom feed
-- links below jump to the message on this page --
2003-08-19 15:53 Some questions Joerg Sommer
This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox;
as well as URLs for NNTP newsgroup(s).