Re: patch: debug instrumentation for dev/raidframe/rf_netbsdkintf.c

[christos%astron.com@localhost (Christos Zoulas)]

In article <20190122173641.GA26414%irregular-apocalypse.k.bsd.de@localhost>,
Christoph Badura  <bad%bsd.de@localhost> wrote:
>On Mon, Jan 21, 2019 at 11:44:04PM +0100, Christoph Badura wrote:
>> On Tue, Jan 22, 2019 at 08:32:48AM +1100, matthew green wrote:
>> > > > @@ -472,6 +472,9 @@
>> > > >  	const char *bootname = device_xname(bdv);
>> > > >  	size_t len = strlen(bootname);
>> > > >  
>> > > > +	if (bdv == NULL)
>> > > > +		return 0;
>> > > > +
>> > > 
>> > > This looked suspicious, even before I read the code.
>> > > 
>> > > The question is if it is ever legitimate for bdv to be NULL.
>> > 
>> > infact, bdv has already been dereferenced at this point:
>> > the assignment to bootname 3 lines up.
>> 
>> Argh.  That's what I get for first removing the code I added which
>> correctly moved the initialization of bootname and len to after the
>> bdv==NULL check and the re-adding it at midnight.
>
>Take 3.  Back to what I intended to submit in the first place.
>
>booted_device is NULL when booting a Xen kernel.  In that case it is OK
>for rf_containsboot() to return 0 because the raid set can't contain "it".
>This matches the checkes for booted_device == NULL near the two
>invocations of rf_containsboot().
>
>I think the check for booted_device == NULL in the last hunk can be
>omitted, but I can't easily test right now.  So I marked it XXX and would
>leave it for later.

LGTM.

christos