cvs commit: src/etc rc.subr src/share/man/man8 rc.subr.8

[Yar Tikhiy]

On Wed, Jun 21, 2006 at 12:56:08PM +0100, Florent Thoumie wrote:
> On Wed, 2006-06-21 at 15:39 +0400, Yar Tikhiy wrote:
> > On Wed, Jun 21, 2006 at 12:05:09PM +0100, Florent Thoumie wrote:
> > > On Wed, 2006-06-21 at 14:52 +0400, Yar Tikhiy wrote:
> > > > On Wed, Jun 21, 2006 at 10:56:25AM +0100, Florent Thoumie wrote:
> > > > > On Wed, 2006-06-21 at 09:42 +0000, Yar Tikhiy wrote:
> > > > > > yar         2006-06-21 09:42:55 UTC
> > > > > > 
> > > > > >   FreeBSD src repository
> > > > > > 
> > > > > >   Modified files:
> > > > > >     etc                  rc.subr 
> > > > > >     share/man/man8       rc.subr.8 
> > > > > >   Log:
> > > > > >   Quite a number of rc.d scripts try to load kernel modules.  Many
> > > > > >   of them do that conditionally depending on kldstat.  The code is
> > > > > >   duplicated all over, but bugs can be uniqie.
> > > > > >   
> > > > > >   To make the things more consistent, introduce a new rc.subr function,
> > > > > >   load_kld, which takes care of loading a kernel module conditionally.
> > > > > >   
> > > > > >   (Found this lying for a while in my p4 branch for various hacks.)
> > > > > 
> > > > > I added such a function some weeks ago (far more simple though). Talking
> > > > > with pjd, I've backed it out to use the somewhat straight-forward method
> > > > > he used in rc.d/geli.
> > > > 
> > > > rc.d/geli doesn't use kldload directlty, so it certainly won't
> > > > benefit from the function I introduced.
> > > 
> > > Then I'm not sure what script would benefit from this function. Can you
> > > point me to an example?
> > 
> > abi
> > archdep
> > atm1
> > hcsecd
> > ipfilter
> > mdconfig
> > mdconfig2
> > pf
> > pflog
> > pfsync
> > sdpd
> > syscons
> > 
> > They all do kldstat then kldload.  Some of them do grep or egrep
> > on kldstat output.  Some of them don't forget to check status from
> > kldload and emit a error message on failure.  Besides, there are
> > scripts that forget to do kldstat in the first place, they just do
> > kldload.  Now all this ado can become just a call to my function.
> 
> Removing all scripts using 'kldstat -q -m foo', we have:

Why should we omit them from our consideration?  The scripts now
doing 'kldstat -q -m foo' can benefit from my function, too, because
they should do error checking and reporting, the error-handling
code being duplicated inconsistently: some of them warn on error,
others print a message on success and so on.  The long line (or its
multi-line equivalent):

	kldstat -q -m foo || kldload foo >/dev/null 2>&1 || echo "Damn! Failed to kldload foo :-("

becomes just:

	load_kld foo

Note that egrep is not used to process such a request in load_kld.

> $ grep kldstat * | grep -v -- "-q -m" | cut -d':' -f1 | sort -u
> abi
> archdep
> atm1
> ipfilter
> syscons
> 
> archdep, atm1 and ipfilter could use this 'kldstat -q -m foo' method, so
> that's only two candidates. Most scripts calling kldload without kldstat
> first could use this method as well.
> 
> But ok, those are definitely scripts I do not read very often.
>
> > > You won't gain anything using grep instead of egrep since they're both
> > > in /usr/bin.
> > 
> > Have I ever tried to?
> 
> I guessed that's what you meant saying "grep is used with -e only, one
> can avoid using it if egrep isn't available yet." What are you planning
> to do then?

I meant -e to my function, not to grep.  That is, if one doesn't
specify -e to load_kld, it won't try to invoke egrep at all.  (FWIW,
grep and egrep are links to the same binary.  And grep -e is not
the same as grep -E ;-)

Without a "-e regex" option, load_kld will just do kldstat.  Therefore,
load_kld won't break scripts that aren't broken yet by using grep
or egrep too early in the boot sequence.

BTW, grep can be emulated with /bin/expr if needed:

_grep()
{
	while read _line; do
		[ `expr "$_line" : ".*$1"` != 0 ] && echo "$_line"
	done
}

Ditto for "grep -q".

-- 
Yar