In article <20190122173641.ga26...@irregular-apocalypse.k.bsd.de>, Christoph Badura <b...@bsd.de> 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
