On Thu, Jan 20, 2011 at 10:19:28PM +0100, Harald Jenny wrote:
> Hello Agustin Martin
>
> > Typical problems with those chained '&&' appear when 'set -e' is used, but
> > amavisd-milter init script does not use it.
>
> Ah ok thanks will keep this info in mind.
Note that ''||' chains are different, will only fail if all components fail.
However, chaining too much also affects readability.
> > When there are more that two elements in the chain, I usually find these
> > if-clauses way more readable and so easier to debug.
>
> I checked with other init scripts an in order to have a consistent coding
> style
> in the init script I replaced the && with if-clauses - could you take a look
> at
> it and tell me if this looks better to you?
It indeed looks better to me, thanks. While I did not test myself I think
you may have problems with empty $MILTERSOCKET with dash as interpreter in
if [ $MILTERSOCKET -a "`echo $MILTERSOCKET | grep -v ^inet`" ]
as in e.g.,
----8<---- test
#!/bin/sh
bs="bs"
if [ $as -a $bs ]; then
echo "Hi"
fi
----8<--- end test
$ dash test
[: 5: -a: unexpected operator
Quoting variables should help.
Also I am not sure of full portability of -a there (although seems to not be
a problem with dash). Maintainer scripts normally use chained '&&' in the
if clause (note that this is different from standalone '&&' chains and is
not a problem under 'set -e')
I'd write above line as
if [ "$MILTERSOCKET" ] && [ "`echo $MILTERSOCKET | grep -v ^inet`" ]; then
but as Teodor points out (just read it), second check seems to be enough.
Also, I'd do a more extensive variable quoting,
if [ "$MILTERSOCKET" ]; then
if [ "$AMAVISSOCKET" ]; then
...
besides those Teodor points out.
--
Agustin
--
To UNSUBSCRIBE, email to debian-bugs-rc-requ...@lists.debian.org
with a subject of "unsubscribe". Trouble? Contact listmas...@lists.debian.org
