Hi Teodor Micu > I had a look at the new patch and I have a few observations.
Thanks > 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 Ok will look at it. > > 2) This construction doesn't work if PIDFILE is empty (not defined): > +if [ $PIDFILE != "/var/run/amavis/$NAME.pid" ]; then Correct so I will check this too. > > For all strings comparison you need to use "$VAR". Good point, missed this. > > 3) This seems to be valid but I've always used [ -n "$VAR" ] instead of: > +if [ $MILTERSOCKET ]; then Will look at this too. > > 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. Agreed, will be fixed. > > 5) This has a mistake: and extra ) character at the end > + chown $USER:$(id $USER -g -n) $(dirname $PIDFILE)) Yes seems I missed this too... > > The correct execution would be: > + chown $USER:$(id $USER -g -n) "$(dirname "$PIDFILE")" Thanks for this hint. > > 6) I'm not sure this is working properly, I've always used [ -n "" -a.. ]. > +if [ $MILTERSOCKET -a "`echo..." ]; then Ok > > Also, there is no need to check if MILTERSOCKET is empty in this case. I tend to disagree here as MILTERSOCKET can be defined in /etc/default/amavisd-milter (this is rather a guard against misconfiguration here). > > 7) You should probably add "-q" for all these executions to avoid > unwanted strings during start/stop/restart. > "`echo $MILTERSOCKET | grep -v ^inet`" If MILTERSOCKET is checked to contain text too? > > 8) The same as #5 for this construction: > + if [ ! -e $(dirname $MILTERSOCKET) ]; then > + mkdir $(dirname $MILTERSOCKET) > + fi > + chown $USER $(dirname $MILTERSOCKET)) Ok > > It is unclear to me why you need to change the owner of MILTERSOCKET. Because amavisd-milter running as user can't otherwise remove the socket when being stopped. > > 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 I will post a new patch later and would really appreciate if you could take a look at it. > > Thanks > Thanks Harald Jenny -- To UNSUBSCRIBE, email to debian-bugs-rc-requ...@lists.debian.org with a subject of "unsubscribe". Trouble? Contact listmas...@lists.debian.org
