Date: Fri, 19 Oct 2007 21:27:58 +0300 From: Mike Makonnen <mtm@FreeBSD.Org> To: John E Hein <jhein@timing.com> Cc: freebsd-rc@FreeBSD.Org Subject: Re: rc.subr, 1.34.2.22, breaks amd_map_program="ypcat -k amd.master" in RELENG_6 Message-ID: <20071019182757.GA38833@terra.mike.lan> In-Reply-To: <18200.45690.542280.695857@gromit.timing.com> References: <18199.34219.154950.645190@gromit.timing.com> <18199.44324.813707.124793@gromit.timing.com> <20071019085154.GA3185@terra.mike.lan> <18200.45690.542280.695857@gromit.timing.com>
next in thread | previous in thread | raw e-mail | index | archive | help
On Fri, Oct 19, 2007 at 07:34:50AM -0600, John E Hein wrote: > > 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). I'm not sure that it is needed. In fact, I would normally assume that any arguments would *not* contain newlines (only spaces) as argument delimiters. As far as I am concerned, the fact rc.d/amd worked up until this change is purely coincidental. In other words, the bug was in the original implementation. It should be up to the script to do whatever cleanup is needed on its input and try to pass sane values to the rc.subr(8) routines. Going the other way and trying to make the run_rc_command() routine permissive in what it accepts is probably asking for trouble. > > 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 ;). I think you're right. I'll take a look at it. [snip patch] > > 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. It's a matter of correctness. The rc_flags variable should contain only arguments for the program. Any shell directives or extra switches the script appends should go in command_args. But you're right, it's a separate issue. Cheers. -- Mike Makonnen | GPG-KEY: http://people.freebsd.org/~mtm/mtm.asc mmakonnen @ gmail.com | AC7B 5672 2D11 F4D0 EBF8 5279 5359 2B82 7CD4 1F55 mtm @ FreeBSD.Org | FreeBSD - http://www.freebsd.org
Want to link to this message? Use this URL: <https://mail-archive.FreeBSD.org/cgi/mid.cgi?20071019182757.GA38833>