On Mon, Jan 24, 2011 at 07:22:17PM +0200, Teodor MICU wrote:
> 2011/1/24 Harald Jenny <har...@a-little-linux-box.at>:
> > On Mon, Jan 24, 2011 at 06:37:37PM +0200, Teodor MICU wrote:
> >> 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"
>
> Yes.
Ok although the PIDFILE line can be removed with the below code.
>
> >> 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.
>
> I don't think this is a supported configuration. I mean $NAME should
> probably not be changed in config from /etc/default but as you say it
> is good to be on the safe side.
Yes *ggg*
>
> >> 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?
>
> Yes. It should be a valid config if /etc/default/$NAME that doesn't
> contain anything. Actually it should be the default to have only
> comments with the default values that the SysAdmin should be able to
> change with custom values. I don't know what you have now in the
> default config.
Well it's actually this way but PIDFILE needs to be set explicitly for the
--pidfile option in the start-stop-daemon stanza - the binary itself already
has the default path hardcoded.
>
> One more important issue I think we missed so far is to have
> MILTERSOCKET empty in the config file. I think you covered only to
> skip "chmod+chown" but in fact it should abort execution. So this
> construction:
>
> +if [ -n "$MILTERSOCKET" ]; then
>
> should be change to:
>
> | if [ -z "$MILTERSOCKET" ]; then
> | echo ".. abort"
> | exit 1
> | else
> [..]
Not really as there is a default value set in the binary which points to
/var/lib/amavis/amavisd-milter.sock - I guess I should rather do:
if [ -z "$MILTERSOCKET" ]; then
MILTERSOCKETTYPE=pipe
else
OPTIONS="$OPTIONS -s $MILTERSOCKET"
if [ "$(echo $MILTERSOCKET | grep ^inet)" ]; then
MILTERSOCKETTYPE=tcp
else
MILTERSOCKETTYPE=pipe
if [ ! -e "$(dirname "$MILTERSOCKET")" ]; then
mkdir "$(dirname "$MILTERSOCKET")"
fi
chown "$USER" "$(dirname "$MILTERSOCKET")"
fi
fi
as this would allow to also set MILTERSOCKETOWNER and MILTERSOCKETMODE with the
default pipe location - what do you think?
>
> Thanks
>
Thanks
Harald
--
To UNSUBSCRIBE, email to debian-bugs-rc-requ...@lists.debian.org
with a subject of "unsubscribe". Trouble? Contact listmas...@lists.debian.org
