Date: Sun, 10 Jun 2012 00:06:57 -0400 From: Sahil Tandon <sahil@tandon.net> To: Doug Barton <dougb@FreeBSD.org> Cc: freebsd-rc@freebsd.org Subject: Re: RESEND: [sahil@tandon.net: Request for review: mail/postfix-postfwd rc script] Message-ID: <20120610040657.GA1415@magic.hamla.org> In-Reply-To: <4FD38ED0.7070803@FreeBSD.org> References: <20120609010405.GA295@magic.hamla.org> <4FD38ED0.7070803@FreeBSD.org>
next in thread | previous in thread | raw e-mail | index | archive | help
--liOOAslEiF7prFVr
Content-Type: text/plain; charset=us-ascii
Content-Disposition: inline
On Sat, 2012-06-09 at 10:58:40 -0700, Doug Barton wrote:
> ...
> 1. Add a $FreeBSD$
> 2. Remove some trailing whitespace
Cool, thanks for that as well as your other comments, to which I reply
inline:
> 3. pidfile= is magic, and needs to be included. It's also not usually
> necessary to allow the user to change the pidfile location, but if you
> really want to allow that, the way to do it is in the new patch.
My (evidently inaccurate) read of /etc/rc.subr led me to believe that
the pidfile global was not strictly required, especially because its
magical qualities appear to be lacking in this case. :) Indeed, that is
why I had to add the status_cmd kludge and - I just realized - another
one for reload_cmd.
> 4. It's not clear to me why there are so many items in the default
> _flags option. Are any of those truly required? If so, they should be in
> command_args. OTOH, if what this represents is a rational default
> configuration that the user might want to twiddle, it's fine.
You're spot on with the OTOH; that is the rationale.
> 5. I used the same trick for the conf file as I did for pidfile
> (utilizing required_files). It was already properly utilized in
> command_args in your patch.
> 6. Rather than having stop_cmd and stop_postcmd be separate functions I
> just put them in line.
Elegant, thank you.
> 7. With pidfile= the check command you added should not be necessary,
> 'service postfwd status' is what you want instead.
That does not work, because of the peculiarities of this program:
looking for its procname (with or without interpreter) is futile because
the ps output shows '[perl5.10.1]' rather than something more useful. :(
> Of course, none of this is tested, so you'll want to do that. :)
> ...
One thing I retained is start_precmd, which is needed because of an
upstream bug (I've reported it) which causes the program to silently
terminate if 'start' is issued when postfwd is already running! Let me
know if there is a more suitable way to handle this until it is fixed
upstream.
I've attached a new patch -- let me know if it gets mangled again in
transit!
--
Sahil Tandon
--liOOAslEiF7prFVr
Content-Type: text/plain; charset=us-ascii
Content-Disposition: attachment; filename="postfwd.in.patch-new"
Index: files/postfwd.in
===================================================================
RCS file: /home/pcvs/ports/mail/postfix-postfwd/files/postfwd.in,v
retrieving revision 1.4
diff -u -u -r1.4 postfwd.in
--- files/postfwd.in 14 Jan 2012 08:56:01 -0000 1.4
+++ files/postfwd.in 10 Jun 2012 04:01:20 -0000
@@ -1,45 +1,63 @@
#!/bin/sh
-# PROVIDE: postfwd
+# $FreeBSD$
+#
+# PROVIDE: postfwd
# REQUIRE: LOGIN cleanvar
# KEYWORD: shutdown
#
# Add the following lines to /etc/rc.conf.local or /etc/rc.conf
# to enable this service:
#
-# postfwd_enable (bool):
+# postfwd_enable (bool):
# Set to "NO" by default.
-# Set it to "YES" to enable postfwd.
-# postfwd_config (path): Set to %%PREFIX%%/etc/postfwd.conf
-# by default.
-#
+# Set it to "YES" to enable postfwd.
+# postfwd_config (path):
+# Set to %%PREFIX%%/etc/postfwd.conf
+# by default.
. /etc/rc.subr
name=postfwd
rcvar=postfwd_enable
-command=%%PREFIX%%/bin/${name}
-required_files=%%PREFIX%%/etc/${name}.conf
-pidfile="/var/run/${name}.pid"
+load_rc_config $name
-stop_postcmd=stop_postcmd
+: ${postfwd_enable:="NO"}
+: ${postfwd_flags="--shortlog --summary=600 --cache=600 --cache-rbl-timeout=3600 --cleanup-requests=1200 --cleanup-rbls=1800 --cleanup-rates=1200"}
-stop_postcmd()
-{
- rm -f $pidfile
-}
+pidfile=${postfwd_pidfile:="/var/run/${name}.pid"}
+required_files=${postfwd_config:="%%PREFIX%%/etc/${name}.conf"}
-load_rc_config "$name"
+command=%%PREFIX%%/bin/${name}
+command_args="-d -f ${required_files} --pidfile=${pidfile} -i 127.0.0.1 -p 10040 -u nobody -g nobody"
-case "$postfwd_enable" in
- [Yy][Ee][Ss] | 1 | [Oo][Nn] | [Tt][Rr][Uu][Ee]) ;;
- *) echo "To make use of $name you must first set $rcvar=\"YES\" in /etc/rc.conf" ;;
-esac
+start_precmd="${name}_check"
+status_cmd="${name}_status"
+stop_cmd="${command} -k --pidfile=${pidfile}"
+stop_postcmd="rm -f ${pidfile}"
+extra_commands="reload"
+reload_cmd="${name}_reload"
+
+postfwd_check() {
+ if [ -f "${postfwd_pidfile}" ]; then
+ err 1 "${name} is already running."
+ fi
+}
+
+postfwd_status() {
+ postfwd_pid=`cat ${pidfile} 2>/dev/null`
+ postfwd_run=`ps -U nobody | grep -m 1 ${postfwd_pid} 2>/dev/null`
+ if [ -n "${postfwd_pid}" -a -n "${postfwd_run}" ]; then
+ echo "$name is running as ${postfwd_pid}"
+ else
+ echo "$name is not running"
+ fi
+}
-: ${postfwd_enable="NO"}
-: ${postfwd_config="%%PREFIX%%/etc/${name}.conf"}
+postfwd_reload() {
-command_args="--shortlog --summary=600 --cache=600 --cache-rbl-timeout=3600 --cleanup-requests=1200 --cleanup-rbls=1800 --cleanup-rates=1200 -d -f ${required_files} -i 127.0.0.1 -p 10040 -u nobody -g nobody"
+ kill -HUP `cat $pidfile`
+}
run_rc_command "$1"
--liOOAslEiF7prFVr--
Want to link to this message? Use this URL: <https://mail-archive.FreeBSD.org/cgi/mid.cgi?20120610040657.GA1415>