[Harald please CC: my address otherwise I could not see your responses]

2011/1/20 Harald Jenny <har...@a-little-linux-box.at>:
> 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?

I had a look at the new patch and I have a few observations.

1) This construction is fine and simple as recommended in /etc/init.d/skeleton:
# Read configuration variable file if it is present
[ -r /etc/default/$NAME ] && . /etc/default/$NAME

2) This construction doesn't work if PIDFILE is empty (not defined):
+if [ $PIDFILE != "/var/run/amavis/$NAME.pid" ]; then

For all strings comparison you need to use "$VAR".

3) This seems to be valid but I've always used [ -n "$VAR" ] instead of:
+if [ $MILTERSOCKET ]; then

4) This doesn't work if the directory has a space:
+  if [ ! -e $(dirname $PIDFILE) ]; then
+    mkdir $(dirname $PIDFILE)

You need to use "$(dirname "$PIDFILE")". This is an issue if this
variable can be defined to any custom value.

5) This has a mistake: and extra ) character at the end
+  chown $USER:$(id $USER -g -n) $(dirname $PIDFILE))

The correct execution would be:
+  chown $USER:$(id $USER -g -n) "$(dirname "$PIDFILE")"

6) I'm not sure this is working properly, I've always used [ -n "" -a.. ].
+if [ $MILTERSOCKET -a "`echo..." ]; then

Also, there is no need to check if MILTERSOCKET is empty in this case.

7) You should probably add "-q" for all these executions to avoid
unwanted strings during start/stop/restart.
  "`echo $MILTERSOCKET | grep -v ^inet`"

8) The same as #5 for this construction:
+  if [ ! -e $(dirname $MILTERSOCKET) ]; then
+    mkdir $(dirname $MILTERSOCKET)
+  fi
+  chown $USER $(dirname $MILTERSOCKET))

It is unclear to me why you need to change the owner of MILTERSOCKET.

Let me know if you need some help to fix this bug. I'm not using
amavisd anymore but I could help in this case.

Thanks



-- 
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