Date: Tue, 28 Mar 2006 09:08:42 -0800 From: Brooks Davis <brooks@one-eyed-alien.net> To: Florent Thoumie <flz@xbsd.org> Cc: freebsd-rc@freebsd.org Subject: Re: rc.subr / rc.d/sshd patch for review Message-ID: <20060328170842.GA16561@odin.ac.hmc.edu> In-Reply-To: <1143556715.65237.4.camel@mayday.esat.net> References: <1143202549.16053.145.camel@mayday.esat.net> <20060324205627.GA18100@odin.ac.hmc.edu> <00E087F1-81E4-4580-A655-50F3DD8A471F@xbsd.org> <1143461191.4290.5.camel@mayday.esat.net> <20060327183745.GA19473@odin.ac.hmc.edu> <1143556715.65237.4.camel@mayday.esat.net>
next in thread | previous in thread | raw e-mail | index | archive | help
--82I3+IH0IqGh5yIs
Content-Type: text/plain; charset=us-ascii
Content-Disposition: inline
Content-Transfer-Encoding: quoted-printable
On Tue, Mar 28, 2006 at 03:38:35PM +0100, Florent Thoumie wrote:
> On Mon, 2006-03-27 at 10:37 -0800, Brooks Davis wrote:
> > 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:
> > > >=20
> > > > > 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 =
=20
> > > > >> 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 =
=20
> > > > >> (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 =20
> > > > > entierly
> > > > > safe. If it's not set, should we try to guess prefix from $0?
> > > >=20
> > > > Somehow, command gets set to the right value, but you're right, I'm=
=20
> > > > missing a bit here.
> > >=20
> > > Hum, re-reading rc.subr, you were right, so I just did what you
> > > supposed.
> >=20
> > Thinking about this a bit more, in the guessing frmo $0 case, your prop=
osed
> > code:
> >=20
> > + prefix=3D${0%/etc/rc.d/*}/
> >=20
> > won't work reliably when the user uses a relative path. I think somethi=
ng
> > like this would be better:
> >=20
> > _tmp=3D`/bin/realpath $0`
> > prefix=3D${_tmp%/etc/rc.d/*}/
>=20
> Indeed, fixed.
>=20
> > > > > 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}" !=3D "/etc" -a -f ${etcdir}/=20
> > > > > 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.
> > > >=20
> > > > Fair enough, but I'd like to add a note saying that /etc/rc.conf.d/=
$=20
> > > > {name} is deprecated for ${etcdir} !=3D "/etc".
> >=20
> > 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} !=3D /etc.
>=20
> Next time I'll test my changes (and sleep more).
>=20
> Did that too, and added a check to test if there's a
> ${etcdir}/rc.conf.d/${_command} file.
Testing prefix=3D/ isn't sufficent since prefix could also be /usr. You
should check etcdir=3D/etc.
I don't think there's much point in the second test. I don't like silent
ignoring of files, it's really hard to debug. Instead, I'd source the file
in that case, but print a warning that two files exist.
> Patch updated : http://people.freebsd.org/~flz/local/rc.d-sshd.diff
>=20
> BTW, I think that we should s/_command/_name/ in load_rc_config(), this
> is a bit confusing.
That sounds reasonable.
-- Brooks
--=20
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
--82I3+IH0IqGh5yIs
Content-Type: application/pgp-signature
Content-Disposition: inline
-----BEGIN PGP SIGNATURE-----
Version: GnuPG v1.2.1 (GNU/Linux)
iD8DBQFEKW2ZXY6L6fI4GtQRAr3QAKDbNJOQexY58hnMcaToOhaoKr8x3ACg1gUT
abDIkLNY/5+If4Y3EoeLp1s=
=Nz23
-----END PGP SIGNATURE-----
--82I3+IH0IqGh5yIs--
Want to link to this message? Use this URL: <https://mail-archive.FreeBSD.org/cgi/mid.cgi?20060328170842.GA16561>