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-dist-requ...@lists.debian.org
with a subject of "unsubscribe". Trouble? Contact listmas...@lists.debian.org

Reply via email to