From mboxrd@z Thu Jan 1 00:00:00 1970 From: Joerg Sommer Date: Tue, 19 Aug 2003 15:53:48 +0000 Subject: Some questions Message-Id: List-Id: MIME-Version: 1.0 Content-Type: text/plain; charset="iso-8859-1" Content-Transfer-Encoding: quoted-printable To: linux-hotplug@vger.kernel.org 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/hotpl= ug --- hotplug-2003_08_05.orig/sbin/hotplug 2003-06-28 02:13:10.0000000= 00 +0 +++ hotplug-2003_08_05/sbin/hotplug 2003-08-19 16:21:38.000000000 +0200 @@ -28,8 +28,8 @@ DIR=3D"/etc/hotplug.d" =20 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 =20 #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 hotp= lug-2003_08_05/etc/hotplug.d/default/default.hotplug --- hotplug-2003_08_05.orig/etc/hotplug.d/default/default.hotplug 2003-05-0= 2 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 $ # =20 -cd /etc/hotplug -. hotplug.functions +set -e + +# export DEBUG=3Dyes =20 -# DEBUG=3Dyes export DEBUG +cd /etc/hotplug +. ./hotplug.functions =20 debug_mesg "arguments ($*) env (`env`)" =20 @@ -38,19 +40,16 @@ # through argv. #=20 if [ $# -lt 1 -o "$1" =3D "help" -o "$1" =3D "--help" ]; then - if [ -t ]; then + if [ -t 1 ]; then echo "Usage: $0 AgentName [AgentArguments]" =20 - AGENTS=3D"" - for AGENT in /etc/hotplug/*.agent ; do - TYPE=3D`basename $AGENT | sed s/.agent//` + printf "AgentName values on this system:" + for AGENT in $HOTPLUG_DIR/*.agent ; do if [ -x $AGENT ]; then - AGENTS=3D"$AGENTS $TYPE" - else - AGENTS=3D"$AGENTS ($TYPE)" + printf " `basename $AGENT .agent`" fi done - echo "AgentName values on this system: $AGENTS"=20 + echo else mesg "illegal usage $*" fi @@ -61,12 +60,10 @@ # Delegate event handling: # /sbin/hotplug FOO ..args.. =3D=3D> /etc/hotplug/FOO.agent ..args.. # -AGENT=3D/etc/hotplug/$1.agent +AGENT=3D$HOTPLUG_DIR/$1.agent if [ -x $AGENT ]; then shift - if [ "$DEBUG" !=3D "" ]; 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.00= 0000000 +0200 @@ -10,7 +10,7 @@ # # =20 -# DEBUG=3Dyes; export DEBUG +# export DEBUG=3Dyes PATH=3D/bin:/sbin:/usr/sbin:/usr/bin =20 KERNEL=3D`uname -r` @@ -22,31 +22,31 @@ . /etc/sysconfig/hotplug fi =20 -if [ -x /usr/bin/logger ]; then - LOGGER=3D/usr/bin/logger -elif [ -x /bin/logger ]; then - LOGGER=3D/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" =3D yes ]; then + debug_mesg () { + echo "$@" + } + else + debug_mesg () { + : + } + fi else mesg () { - $LOGGER -t $(basename $0)"[$$]" "$@" + logger $(basename $0)"[$$] $@" + } + debug_mesg () { + logger $(basename $0)"[$$] $@" } fi =20 -debug_mesg () { - test "$DEBUG" =3D "" -o "$DEBUG" =3D 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=3D"" - # make this routine more readable TYPE=3D$1 FILENAME=3D$2 DESCRIPTION=3D$3 =20 + DRIVERS=3D"" # 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=3D`type -p ${TYPE}modules` ;; - *) LISTER=3D"" ;; - esac - - if [ "$LISTER" !=3D "" ]; then + 2.2*|2.3*|2.4*) + LISTER=3D`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" =3D "" -o ! -f "$DEVICE" ]; then - LISTER=3D - else + if [ -f "$DEVICE" ]; then DRIVERS=3D`$LISTER --mapfile $FILENAME --device $DEVICE` - fi ;; - + fi + ;; pci) debug_mesg "pcimodules is scanning more than $PCI_SLOT ..." DRIVERS=3D`$LISTER` ;; esac - fi + ;; + esac =20 # try parsing by shell scripts if no luck yet - if [ "$DRIVERS" =3D "" ]; then + if [ -z "$DRIVERS" ]; then ${TYPE}_map_modules < $FILENAME fi =20 # FIXME remove dups and blacklisted modules from $DRIVERS here =20 - if [ "$DRIVERS" =3D "" ]; then + if [ -z "$DRIVERS" ]; then return fi =20 @@ -139,9 +133,8 @@ do # maybe driver modules need loading LOADED=3Dfalse - 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=3Dtrue + $HOTPLUG_DIR/$TYPE/$MODULE && LOADED=3Dtrue fi =20 if [ $LOADED =3D false ]; then #v- Bye, Joerg. --=20 =BBPerl - the only language that looks the same before and after RSA encryption.=AB -- 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=104 _______________________________________________ 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