From owner-cvs-all@FreeBSD.ORG Thu Nov 25 19:05:29 2010 Return-Path: Delivered-To: cvs-all@FreeBSD.org Received: from mx1.freebsd.org (mx1.freebsd.org [IPv6:2001:4f8:fff6::34]) by hub.freebsd.org (Postfix) with ESMTP id C1404106566B for ; Thu, 25 Nov 2010 19:05:29 +0000 (UTC) (envelope-from dougb@FreeBSD.org) Received: from mail2.fluidhosting.com (mx23.fluidhosting.com [204.14.89.6]) by mx1.freebsd.org (Postfix) with ESMTP id 6233A8FC14 for ; Thu, 25 Nov 2010 19:05:29 +0000 (UTC) Received: (qmail 1114 invoked by uid 399); 25 Nov 2010 19:05:28 -0000 Received: from localhost (HELO doug-optiplex.ka9q.net) (dougb@dougbarton.us@127.0.0.1) by localhost with ESMTPAM; 25 Nov 2010 19:05:28 -0000 X-Originating-IP: 127.0.0.1 X-Sender: dougb@dougbarton.us Message-ID: <4CEEB376.9030701@FreeBSD.org> Date: Thu, 25 Nov 2010 11:05:26 -0800 From: Doug Barton Organization: http://SupersetSolutions.com/ User-Agent: Mozilla/5.0 (X11; U; FreeBSD amd64; en-US; rv:1.9.2.12) Gecko/20101028 Thunderbird/3.1.6 MIME-Version: 1.0 To: Josh Paetzel References: <201011251458.oAPEwX25045477@repoman.freebsd.org> In-Reply-To: <201011251458.oAPEwX25045477@repoman.freebsd.org> X-Enigmail-Version: 1.1.2 OpenPGP: id=1A1ABC84 Content-Type: multipart/mixed; boundary="------------050608010303050907050708" Cc: Tom Judge , cvs-ports@FreeBSD.org, cvs-all@FreeBSD.org, ports-committers@FreeBSD.org Subject: Re: cvs commit: ports/net-mgmt/softflowd/files softflowd.in ports/net-mgmt/softflowd Makefile X-BeenThere: cvs-all@freebsd.org X-Mailman-Version: 2.1.5 Precedence: list List-Id: **OBSOLETE** CVS commit messages for the entire tree List-Unsubscribe: , List-Archive: List-Post: List-Help: List-Subscribe: , X-List-Received-Date: Thu, 25 Nov 2010 19:05:29 -0000 This is a multi-part message in MIME format. --------------050608010303050907050708 Content-Type: text/plain; charset=ISO-8859-1; format=flowed Content-Transfer-Encoding: 7bit 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 > 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/ --------------050608010303050907050708 Content-Type: text/plain; name="softflowd.patch" Content-Transfer-Encoding: 7bit Content-Disposition: attachment; filename="softflowd.patch" 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" - - - --------------050608010303050907050708--