rc.subr, 1.34.2.22, breaks amd_map_program="ypcat -k amd.master" in RELENG_6

[John E Hein]

Mike Makonnen wrote at 21:27 +0300 on Oct 19, 2007:
 > On Fri, Oct 19, 2007 at 07:34:50AM -0600, John E Hein wrote:
 > > 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.

It can't hurt to put some hints in the comments in rc.subr what sane
values are (i.e., "no newlines, please" ;).  But since this is pretty
abnormal, I'm not going to lose sleep if it's not documented.


 > Going the other way and trying to make the run_rc_command() routine
 > permissive in what it accepts is probably asking for trouble.

Agreed.


 > > 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.

Thanks.


 > 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.

Okay.  A subtle distinction, but I see it now in the comments in rc.subr