On Mon, Jan 24, 2011 at 08:59:41PM +0200, Teodor MICU wrote: > 2011/1/24 Harald Jenny <har...@a-little-linux-box.at>: > > Ok although the PIDFILE line can be removed with the below code. > > I'm don't see where PIDFILE is removed.
Just take a look at the next patch version. > > >> 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: > > I was wrong here. MILTERSOCKET can be empty in the config from > /etc/default/$NAME and this should be the default. Yes but if it's empty the default value is a unix socket. > > > 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 > > [..] > > fi > > > > as this would allow to also set MILTERSOCKETOWNER and MILTERSOCKETMODE with > > the > > default pipe location - what do you think? > > I think the above test is always false if you correctly initialize > MILTERSOCKET after the /etc/default/$NAME has been sourced. > [ -z "$MILTERSOCKET" ] || MILTERSOCKET="/var/lib/amavis/amavisd-milter.sock" True but I only want to append to OPTIONS when the default values are overriden because this keeps the command line shorter :-). > > Later on you don't check if it's empty but only the type: unix vs inet. Well seems to me this is just a change in the code flow... > > Thanks > Thanks for your feedback
--- /etc/init.d/amavisd-milter 2010-05-12 23:01:42.000000000 +0200 +++ /etc/init.d/amavisd-milter 2011-01-24 20:47:02.000000000 +0100 @@ -25,26 +25,48 @@ DAEMON=/usr/sbin/amavisd-milter NAME=amavisd-milter DESC="amavisd-milter Daemon" -USER=amavis -PIDFILE=/var/run/amavis/$NAME.pid OPTIONS="" # Exit if the package is not installed -[ -x $DAEMON ] || exit 0 +[ -x "$DAEMON" ] || exit 0 # Read configuration variable file if it is present -[ -r /etc/default/$NAME ] && . /etc/default/$NAME +[ -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" +[ -n "$SYSTEMUSER" ] || SYSTEMUSER=amavis -[ $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 [ -z "$PIDFILE" ]; then + PIDFILE="/var/run/amavis/$NAME.pid" +else + OPTIONS="$OPTIONS -p $PIDFILE" +fi +PIDDIR="$(dirname "$PIDFILE")" +if [ ! -e "$PIDDIR" ]; then + mkdir "$PIDDIR" + chown "$SYSTEMUSER":"$(id "$SYSTEMUSER" -g -n)" "$PIDDIR" +fi + +if [ -z "$MILTERSOCKET" ]; then + MILTERSOCKETTYPE=pipe +else + OPTIONS="$OPTIONS -s $MILTERSOCKET" + if [ "$(echo $MILTERSOCKET | grep ^inet)" ]; then + MILTERSOCKETTYPE=tcp + else + MILTERSOCKETTYPE=pipe + MILTERDIR="$(dirname "$MILTERSOCKET")" + if [ ! -e "$MILTERDIR" ]; then + mkdir "$MILTERDIR" + chown "$SYSTEMUSER" "$MILTERDIR" + fi + fi +fi + +[ -z "$AMAVISSOCKET" ] || OPTIONS="$OPTIONS -S $AMAVISSOCKET" +[ -z "$WORKINGDIR" ] || OPTIONS="$OPTIONS -w $WORKINGDIR" +[ -z "$EXTRAPARAMS" ] || OPTIONS="$OPTIONS $EXTRAPARAMS" -START="--start --quiet --chuid $USER --pidfile $PIDFILE --startas $DAEMON --name $NAME -- $OPTIONS" +START="--start --quiet --chuid $SYSTEMUSER --pidfile $PIDFILE --startas $DAEMON --name $NAME -- $OPTIONS" STOP="--stop --quiet --retry 10 --pidfile $PIDFILE --startas $DAEMON --name $NAME" # Define LSB functions. @@ -53,25 +75,50 @@ 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 [ "$MILTERSOCKETTYPE" = "pipe" ]; 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 ;;
