On Mon, Jan 24, 2011 at 06:37:37PM +0200, Teodor MICU wrote: > Hi again,
Hello again :-) > > 2011/1/24 Harald Jenny <har...@a-little-linux-box.at>: > > first thanks to everbody for the valuable input, it helped me a lot to > > improve > > this init script. Please take a look at the third version of my patch and > > comment on it. > > Overall it seems fine, just a few observations: > > 1) usually you should enclose with "" the full path here: > +PIDFILE=/var/run/amavis/"$NAME".pid > +[ -r /etc/default/"$NAME" ] && . /etc/default/"$NAME" You mean like this? PIDFILE="/var/run/amavis/$NAME.pid" [ -r "/etc/default/$NAME" ] && . "/etc/default/$NAME" > > Also, $NAME is not usually quoted with "" in any init script. Well that's true but you may set it to a value with blanks in it too so maybe the defaults should also be changed here? If you think it's not necessary I may also drop in but I wanted to be on the safe side here. > > 2) You should make sure that you don't get it this situation: > +if [ -n "$PIDFILE" ]; then > ... > +else > + log_failure_msg "Error: PIDFILE variable must be defined for > correct functionality" > + exit 1 > +fi > > This is usually done by setting the default values (for example for > PIDFILE) after you have sourced the config from /etc/default/$NAME. > This is done on a test like this: > test -z "$VAR" || VAR="<your default value>" > (I don't recommend "test -n $VAR && .. not that good with "set -e") You mean not alerting when PIDFILE is set to "" in the config file but rather changing it to my default value, correct? > > You should consider all variables essential to amavisd-milter startup > and runtime. > > 3) Replace these to work with "set -e": > +[ -n "$AMAVISSOCKET" ] && .. > with > +[ -z "$AMAVISSOCKET" ] || .. Hmmm ok > > Thanks Thank you Harald -- To UNSUBSCRIBE, email to debian-bugs-rc-requ...@lists.debian.org with a subject of "unsubscribe". Trouble? Contact listmas...@lists.debian.org
