rc.subr / rc.d/sshd patch for review
Brooks Davis
brooks at one-eyed-alien.net
Mon Mar 27 18:37:46 UTC 2006
On Mon, Mar 27, 2006 at 01:06:30PM +0100, Florent Thoumie wrote:
> On Sat, 2006-03-25 at 11:06 +0000, Florent Thoumie wrote:
> > On Mar 24, 2006, at 8:56 PM, Brooks Davis wrote:
> >
> > > On Fri, Mar 24, 2006 at 12:15:49PM +0000, Florent Thoumie wrote:
> > >> This is based on Oliver's patch for rc.d/sshd that can be found in
> > >> Gnats.
> > >>
> > >> In load_rc_config, I'm extracting prefix from ${command} (or
> > >> ${name}_program, which part is moved from run_rc_command), and
> > >> setting
> > >> etcdir accordingly.
> > >>
> > >> The point is that some scripts (like rc.d/sshd) can be used for base
> > >> sshd as well as ports sshd, and makes possible to source
> > >> ${prefix}/etc/rc.conf.d/${name}.
> > >>
> > >> This patch also documents ${name}_program above run_rc_command
> > >> (though
> > >> it's actually used in load_rc_config).
> > >
> > > Is command always set? I'm pretty sure it isn't so this may not be
> > > entierly
> > > safe. If it's not set, should we try to guess prefix from $0?
> >
> > Somehow, command gets set to the right value, but you're right, I'm
> > missing a bit here.
>
> Hum, re-reading rc.subr, you were right, so I just did what you
> supposed.
Thinking about this a bit more, in the guessing frmo $0 case, your proposed
code:
+ prefix=${0%/etc/rc.d/*}/
won't work reliably when the user uses a relative path. I think something
like this would be better:
_tmp=`/bin/realpath $0`
prefix=${_tmp%/etc/rc.d/*}/
> > > The other issue I see is that instead of:
> > >
> > > if [ -f ${etcdir}/rc.conf.d/"$_command" ]; then
> > > debug "Sourcing ${etcdir}/rc.conf.d/${_command}"
> > > . ${etcdir}/rc.conf.d/"$_command"
> > > fi
> > >
> > > I think we should do:
> > >
> > > if [ -f /etc/rc.conf.d/"$_command" ]; then
> > > debug "Sourcing /etc/rc.conf.d/${_command}"
> > > . /etc/rc.conf.d/"$_command"
> > > fi
> > > if [ "${etcdir}" != "/etc" -a -f ${etcdir}/
> > > rc.conf.d/"$_command" ]; then
> > > debug "Sourcing ${etcdir}/rc.conf.d/${_command}"
> > > . ${etcdir}/rc.conf.d/"$_command"
> > > fi
> > >
> > > That preserves the old behavior while adding support for
> > > ${prefix}/etc/rc.conf.d.
> >
> > Fair enough, but I'd like to add a note saying that /etc/rc.conf.d/$
> > {name} is deprecated for ${etcdir} != "/etc".
The deprecation warning should not be printed in the case that ${etcdir}
is /etc. You should also avoid sourcing the file twice in the /etc
case. The easiest way to do that is probably to make the first case
contingent on ${etcdir} != /etc.
-- Brooks
--
Any statement of the form "X is the one, true Y" is FALSE.
PGP fingerprint 655D 519C 26A7 82E7 2529 9BF0 5D8E 8BE9 F238 1AD4
-------------- next part --------------
A non-text attachment was scrubbed...
Name: not available
Type: application/pgp-signature
Size: 189 bytes
Desc: not available
Url : http://lists.freebsd.org/pipermail/freebsd-rc/attachments/20060327/0d8b8fcc/attachment.pgp
More information about the freebsd-rc
mailing list