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.

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

> Hope this helps,

Yes thanks it's very informative!

> -- 
> Agustin

Kind regards
Harald Jenny
--- /etc/init.d/amavisd-milter	2010-05-12 23:01:42.000000000 +0200
+++ /etc/init.d/amavisd-milter_FIXED	2011-01-20 21:22:45.000000000 +0100
@@ -33,16 +33,38 @@
 [ -x $DAEMON ] || exit 0
 
 # Read configuration variable file if it is present
-[ -r /etc/default/$NAME ] && . /etc/default/$NAME
-
-[ $PIDFILE != "/var/run/amavis/$NAME.pid" ] && OPTIONS="$OPTIONS -p $PIDFILE"
-[ $MILTERSOCKET ] && OPTIONS="$OPTIONS -s $MILTERSOCKET"
-[ $AMAVISSOCKET ] && OPTIONS="$OPTIONS -S $AMAVISSOCKET"
-[ $WORKINGDIR ] && OPTIONS="$OPTIONS -w $WORKINGDIR"
-[ $EXTRAPARAMS ] && OPTIONS="$OPTIONS $EXTRAPARAMS"
-
-[ $PIDFILE ] && ([ -d $(dirname $PIDFILE) ] || mkdir $(dirname $PIDFILE) && chown $USER:$(id $USER -g -n) $(dirname $PIDFILE))
-[ $MILTERSOCKET ] && ([ -d $(dirname $MILTERSOCKET) ] || mkdir $(dirname $MILTERSOCKET) && chown $USER $(dirname $MILTERSOCKET))
+if [ -r /etc/default/$NAME ]; then
+  . /etc/default/$NAME
+fi
+
+if [ $PIDFILE != "/var/run/amavis/$NAME.pid" ]; then
+  OPTIONS="$OPTIONS -p $PIDFILE"
+fi
+if [ $MILTERSOCKET ]; then
+  OPTIONS="$OPTIONS -s $MILTERSOCKET"
+fi
+if [ $AMAVISSOCKET ]; then
+  OPTIONS="$OPTIONS -S $AMAVISSOCKET"
+fi
+if [ $WORKINGDIR ]; then
+  OPTIONS="$OPTIONS -w $WORKINGDIR"
+fi
+if [ $EXTRAPARAMS ]; then
+  OPTIONS="$OPTIONS $EXTRAPARAMS"
+fi
+
+if [ $PIDFILE ]; then
+  if [ ! -e $(dirname $PIDFILE) ]; then
+    mkdir $(dirname $PIDFILE)
+  fi
+  chown $USER:$(id $USER -g -n) $(dirname $PIDFILE))
+fi
+if [ $MILTERSOCKET -a "`echo $MILTERSOCKET | grep -v ^inet`" ]; then
+  if [ ! -e $(dirname $MILTERSOCKET) ]; then
+    mkdir $(dirname $MILTERSOCKET)
+  fi
+  chown $USER $(dirname $MILTERSOCKET))
+fi
 
 START="--start --quiet --chuid $USER --pidfile $PIDFILE --startas $DAEMON --name $NAME -- $OPTIONS"
 STOP="--stop --quiet --retry 10 --pidfile $PIDFILE --startas $DAEMON --name $NAME"
@@ -53,25 +75,51 @@
 case "$1" in
   start)
 	log_daemon_msg "Starting $DESC:" "$NAME"
-	start-stop-daemon $START 
+	start-stop-daemon $START
+
 	case "$?" in
-		0) log_end_msg 0
-		   [ $MILTERSOCKET ] && [ $MILTERSOCKETOWNER ] && chown $MILTERSOCKETOWNER $MILTERSOCKET
-		   [ $MILTERSOCKET ] && [ $MILTERSOCKETMODE ] && chmod $MILTERSOCKETMODE $MILTERSOCKET ;;
-		1) log_progress_msg "already started"
-		   log_end_msg 0 ;;
-		*) log_end_msg $? ;;
+		0)
+		   log_end_msg 0
+		   if [ $MILTERSOCKET -a "`echo $MILTERSOCKET | grep -v ^inet`" ]; then
+		     if [ $MILTERSOCKETOWNER ]; then
+		       chown $MILTERSOCKETOWNER $MILTERSOCKET
+		     fi
+		     if [ $MILTERSOCKETMODE ]; then
+		       chmod $MILTERSOCKETMODE $MILTERSOCKET
+		     fi
+		   fi
+		   ;;
+
+		1) 
+		   log_progress_msg "already started"
+		   log_end_msg 0
+		   ;;
+
+		*) 
+		   log_end_msg $?
+		   ;;
+
 	esac
 	;;
 
   stop)
 	log_daemon_msg "Stopping $DESC:" "$NAME"
 	start-stop-daemon $STOP
+
 	case "$?" in
-		0) log_end_msg 0 ;;
-		1) log_progress_msg "already stopped"
-		   log_end_msg 0 ;;
-		*) log_end_msg $? ;;
+		0)
+		   log_end_msg 0
+		   ;;
+
+		1)
+		   log_progress_msg "already stopped"
+		   log_end_msg 0
+		   ;;
+
+		*)
+		   log_end_msg $?
+		   ;;
+
 	esac
 	;;
 

Reply via email to