cvs commit: ports/net-mgmt/softflowd/files softflowd.in ports/net-mgmt/softflowd Makefile

[Doug Barton]

With all due respect to Tom, this script is not in good shape, and 
should not have been committed in its current form. I've attached a 
patch that corrects the most glaring errors. It would be great if Tom, 
or someone else familiar with this software, could test it ASAP and 
report back.

1. Lots of extraneous whitespace, including newlines at the beginning 
and end of the script, eols; and inconsistent indentation.
2. No $FreeBSD$
3. Unless there is a good reason to do otherwise local scripts should 
use REQUIRE: LOGIN.
4. There should be no quotes around /etc/rc.subr
5. The default values should be set after calling load_rc_config()
6. We do not like any code to be run in rc.d scripts unconditionally.
7. The script is not PREFIX clean.

Once again, I appreciate the effort put in here, but given that a lot of 
rc.d scripts are copied from other scripts (which seems like what was 
done here) we need to be doubly careful with QA so that bad examples 
don't propagate further.


Doug


On 11/25/2010 06:58, Josh Paetzel wrote:
> jpaetzel    2010-11-25 14:58:33 UTC
>
>    FreeBSD ports repository
>
>    Modified files:
>      net-mgmt/softflowd   Makefile
>    Added files:
>      net-mgmt/softflowd/files softflowd.in
>    Log:
>    Add rc.d script to port
>
>    PR:     ports/151398  http://www.FreeBSD.org/cgi/query-pr.cgi?pr=151398
>    Submitted by:   Tom Judge<tom at tomjudge.com>
>    Approved by:    Maintainer timeout
>
>    Revision  Changes    Path
>    1.5       +2 -0      ports/net-mgmt/softflowd/Makefile
>    1.1       +75 -0     ports/net-mgmt/softflowd/files/softflowd.in (new)
>
> http://www.FreeBSD.org/cgi/cvsweb.cgi/ports/net-mgmt/softflowd/Makefile.diff?&r1=1.4&r2=1.5&f=h
> http://www.FreeBSD.org/cgi/cvsweb.cgi/ports/net-mgmt/softflowd/files/softflowd.in
>



-- 

	Nothin' ever doesn't change, but nothin' changes much.
			-- OK Go

	Breadth of IT experience, and depth of knowledge in the DNS.
	Yours for the right price.  :)  http://SupersetSolutions.com/

-------------- next part --------------
Index: softflowd.in
===================================================================
RCS file: /home/pcvs/ports/net-mgmt/softflowd/files/softflowd.in,v
retrieving revision 1.1
diff -u -r1.1 softflowd.in
--- softflowd.in	25 Nov 2010 14:58:33 -0000	1.1
+++ softflowd.in	25 Nov 2010 18:53:52 -0000
@@ -1,11 +1,11 @@
-
 #!/bin/sh
 
-# (c) 2010 Tom Judge 
+# $FreeBSD$
+
+# (c) 2010 Tom Judge
 
 # PROVIDE: softflowd
-# REQUIRE: NETWORKING
-# BEFORE: LOGIN
+# REQUIRE: LOGIN
 # KEYWORD: shutdown
 
 # softflowd_enable="YES"
@@ -19,57 +19,59 @@
 # softflowd_em0_extra_args=""
 # softflowd_em1_extra_args=""
 
-. "/etc/rc.subr"
-
-softflowd_enable=${softflowd_enable:-"NO"}
-softflowd_timeouts="-t maxlife=300"
-softflowd_max_states="16000"
-softflowd_extra_args=""
+. /etc/rc.subr
 
 name=softflowd
 rcvar=`set_rcvar`
-load_rc_config $name
+
+start_precmd="softflowd_precommand"
+stop_precmd="softflowd_precommand"
 stop_cmd="softflowd_stop"
-command="/usr/local/sbin/softflowd"
+command="%%PREFIX%%/sbin/softflowd"
 _pidprefix="/var/run/softflowd"
 
-if [ -n "$2" ]; then
-	profile="$2"
-    pidfile="${_pidprefix}.${profile}.pid"
-    ctlfile="${_pidprefix}.${profile}.ctl"
-    eval apache22_flags="\${apache22_${profile}_flags:-${apache22_flags}}"
-    eval softflowd_collector="\${softflowd_${profile}_collector}"
-    if [ "x${softflowd_collector}" = "x" ]; then
-        echo "ERROR: You must specify a collector to send data to."
-        exit 1
-    fi
-    eval softflowd_timeouts="\${softflowd_${profile}_timeouts:-${softflowd_timeouts}}"
-    eval softflowd_max_states="\${softflowd_${profile}_max_states:-${softflowd_max_states}}"
-    eval softflowd_extra_args="\${softflowd_${profile}_extra_args:-${softflowd_extra_args}}"
-    command_args="-i ${profile} -n ${softflowd_collector} -m ${softflowd_max_states} -p ${pidfile} -c ${ctlfile} ${softflowd_timeouts} ${softflowd_extra_args}"
-
-else
-	if [ "x${softflowd_interfaces}" != "x" -a "x$1" != "x" ]; then
-		for profile in ${softflowd_interfaces}; do
-			echo "===> softflowd profile: ${profile}"
-			/usr/local/etc/rc.d/softflowd $1 ${profile}
-			retcode="$?"
-			if [ "0${retcode}" -ne 0 ]; then
-				failed="${profile} (${retcode}) ${failed:-}"
-			else
-				success="${profile} ${success:-}"
-			fi
-		done
-		exit 0
-	fi
-fi
+load_rc_config $name
 
-softflowd_stop() {
-   /usr/local/sbin/softflowctl -c ${ctlfile} shutdown 
+softflowd_enable=${softflowd_enable:-"NO"}
+softflowd_timeouts="-t maxlife=300"
+softflowd_max_states="16000"
+
+softflowd_precommand ()
+{
+	if [ -n "$2" ]; then
+		profile="$2"
+		pidfile="${_pidprefix}.${profile}.pid"
+		ctlfile="${_pidprefix}.${profile}.ctl"
+		eval apache22_flags="\${apache22_${profile}_flags:-${apache22_flags}}"
+		eval softflowd_collector="\${softflowd_${profile}_collector}"
+		if [ "x${softflowd_collector}" = "x" ]; then
+			echo "ERROR: You must specify a collector to send data to."
+			exit 1
+		fi
+		eval softflowd_timeouts="\${softflowd_${profile}_timeouts:-${softflowd_timeouts}}"
+		eval softflowd_max_states="\${softflowd_${profile}_max_states:-${softflowd_max_states}}"
+		eval softflowd_extra_args="\${softflowd_${profile}_extra_args:-${softflowd_extra_args}}"
+		command_args="-i ${profile} -n ${softflowd_collector} -m ${softflowd_max_states} -p ${pidfile} -c ${ctlfile} ${softflowd_timeouts} ${softflowd_extra_args}"
+	else
+		if [ "x${softflowd_interfaces}" != "x" -a "x$1" != "x" ]; then
+			for profile in ${softflowd_interfaces}; do
+				echo "===> softflowd profile: ${profile}"
+				/usr/local/etc/rc.d/softflowd $1 ${profile}
+				retcode="$?"
+				if [ "0${retcode}" -ne 0 ]; then
+					failed="${profile} (${retcode}) ${failed:-}"
+				else
+					success="${profile} ${success:-}"
+				fi
+			done
+			exit 0
+		fi
+	fi
 }
 
+softflowd_stop()
+{
+	%%PREFIX%%/sbin/softflowctl -c ${ctlfile} shutdown
+}
 
 run_rc_command "$1"
-
-
-