From owner-freebsd-rc@FreeBSD.ORG Fri Oct 19 13:34:56 2007 Return-Path: Delivered-To: freebsd-rc@FreeBSD.Org Received: from mx1.freebsd.org (mx1.freebsd.org [IPv6:2001:4f8:fff6::34]) by hub.freebsd.org (Postfix) with ESMTP id 1DF0C16A419 for ; Fri, 19 Oct 2007 13:34:56 +0000 (UTC) (envelope-from jhein@timing.com) Received: from Daffy.timing.com (w.timing.com [206.168.13.218]) by mx1.freebsd.org (Postfix) with ESMTP id E084813C46E for ; Fri, 19 Oct 2007 13:34:55 +0000 (UTC) (envelope-from jhein@timing.com) Received: from gromit.timing.com (gromit.timing.com [206.168.13.209]) by Daffy.timing.com (8.13.1/8.13.1) with ESMTP id l9JDYt3Q055172; Fri, 19 Oct 2007 07:34:55 -0600 (MDT) (envelope-from jhein@timing.com) Received: from gromit.timing.com (localhost [127.0.0.1]) by gromit.timing.com (8.14.1/8.14.1) with ESMTP id l9JDYogm073265; Fri, 19 Oct 2007 07:34:50 -0600 (MDT) (envelope-from jhein@gromit.timing.com) Received: (from jhein@localhost) by gromit.timing.com (8.14.1/8.14.1/Submit) id l9JDYoZB073262; Fri, 19 Oct 2007 07:34:50 -0600 (MDT) (envelope-from jhein) MIME-Version: 1.0 Content-Type: text/plain; charset=us-ascii Content-Transfer-Encoding: 7bit Message-ID: <18200.45690.542280.695857@gromit.timing.com> Date: Fri, 19 Oct 2007 07:34:50 -0600 From: John E Hein To: Mike Makonnen In-Reply-To: <20071019085154.GA3185@terra.mike.lan> References: <18199.34219.154950.645190@gromit.timing.com> <18199.44324.813707.124793@gromit.timing.com> <20071019085154.GA3185@terra.mike.lan> X-Mailer: VM 7.19 under Emacs 22.0.99.1 X-Virus-Scanned: ClamAV version 0.91.2, clamav-milter version 0.91.2 on Daffy.timing.com X-Virus-Status: Clean Cc: freebsd-rc@FreeBSD.Org Subject: Re: rc.subr, 1.34.2.22, breaks amd_map_program="ypcat -k amd.master" in RELENG_6 X-BeenThere: freebsd-rc@freebsd.org X-Mailman-Version: 2.1.5 Precedence: list List-Id: "Discussion related to /etc/rc.d design and implementation." List-Unsubscribe: , List-Archive: List-Post: List-Help: List-Subscribe: , X-List-Received-Date: Fri, 19 Oct 2007 13:34:56 -0000 Mike Makonnen wrote at 11:51 +0300 on Oct 19, 2007: > On Thu, Oct 18, 2007 at 12:59:48PM -0600, John E Hein wrote: > > The new run_rc_doit() function does this: > > > > + eval "$@" > > > > If you remove the quotes, it starts working again. > > > > Unfortunately, you can have different problems if you don't quote $@ > > (for instance, args with spaces become two args). I don't know if > > yar@ added that to address a specific issue or just to be more > > future-proof in terms of quoting. But it breaks if the command > > or args have a newline. > > > > Maybe it's best to just do the echo in /etc/rc.d/amd that I showed in > > the previous email and document that $rc_flags and $command, etc., should > > not have newlines. Here's that patch again: > > I agree. > I think that modifying rc.d/amd is the proper solution here. It's probably > a bug that it depends on run_rc_command()'s internal behaviour. I agree - it'd be best if run_rc_command could handle whatever you throw at it, but I'm not sure that's practical. If run_rc_command can't have newlines in the vars it uses as arguments, then one way to handle the bug is just to document it clearly in rc.subr (with the rc_flags and command_args comments). A quick glance shows that the /etc/rc.d/syslogd script is the only(?) one that does something similar. But it takes pains to convert newlines to spaces using tr(1) (although I think it's unnecessary to do so in a for loop - so, IMO, that tr(1) is superfluous and could be removed for the efficiency conscious - not tested ;). > Can you try the following slightly different patch? > > Index: etc/rc.d/amd > =================================================================== > RCS file: /home/ncvs/src/etc/rc.d/amd,v > retrieving revision 1.18 > diff -u -r1.18 amd > --- etc/rc.d/amd 18 Oct 2006 15:56:11 -0000 1.18 > +++ etc/rc.d/amd 19 Oct 2007 08:35:36 -0000 > @@ -34,7 +34,7 @@ > [Nn][Oo] | '') > ;; > *) > - rc_flags="${rc_flags} `eval ${amd_map_program}`" > + rc_flags="${rc_flags} `echo $(eval ${amd_map_program})`" > ;; > esac > > @@ -46,7 +46,8 @@ > fi > ;; > *) > - rc_flags="-p ${rc_flags} > /var/run/amd.pid 2> /dev/null" > + rc_flags="-p ${rc_flags}" > + command_args=" > /var/run/amd.pid 2> /dev/null" > ;; > esac > return 0 That works fine, too (don't need the space in front of '>'), but I don't see the benefit to splitting it into two variables/lines. Is it just a preferred style issue? If so, please commit that separately or note it in the commit message so as not to confuse the issue for future cvs miners. Thanks for taking a look. If you want me to send-pr this, just let me know.