Some questions
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+
sbin/hotplug 2003-08-19 16:21:38.000000000 +0200 renamed from sbin/hotplug 2003-06-28 02:13:10.000000000 +0 +2 -2
@@ -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+
etc/hotplug.d/default/default.hotplug +12 -15
@@ -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+
etc/hotplug/hotplug.functions +28 -36
133 lines not on this page · raw message
#v-
Bye, Joerg.
Signature · 13 lines
»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_code4 _______________________________________________ Linux-hotplug-devel mailing list http://linux-hotplug.sourceforge.net [email protected] https://lists.sourceforge.net/lists/listinfo/linux-hotplug-devel