libpciaccess bogus bridge data - commit 2bda5b73

[Jesse Barnes]

On Mon, 6 Dec 2010 11:38:24 -0800
Bryce Harrington <bryce at canonical.com> wrote:

> On Mon, Dec 06, 2010 at 09:35:48AM -0800, Jesse Barnes wrote:
> > On Wed, 1 Dec 2010 18:54:22 -0800
> > Bryce Harrington <bryce at canonical.com> wrote:
> > 
> > > Jesse,
> > > 
> > > I have a question about change 2bda5b73 that you did to libpciaccess
> > > last year, to stop pci_device_get_bridge_buses() from looking at bus
> > > data if the bridge data was not read.
> > > 
> > > A ubuntu bug reporter says that this check is preventing his device from
> > > getting detected properly - see
> > > 
> > >  https://bugs.launchpad.net/ubuntu/+source/libpciaccess/+bug/681207
> > > 
> > > Looking at the code I see that for case 0x04 there's already a check for
> > > NULL, where it attempts to re-read the bridge info, so presumably there
> > > are at least some situations where NULL bridge.pci is expected?
> > > 
> > > What I'm wondering is if the check should be less strict in cases of
> > > multi-function cards, or if somewhere else in the code needs updated to
> > > read these devices properly.
> > 
> > The multifunction check is probably ok, but there should still be a
> > bridge above the device regardless, even if it's just a host bridge I
> > think, so we should figure out why that's not getting set...
> 
> In the discussion on 681207, it seems the portion of the data structure
> being tested by this patch for null is always going to be null when it's
> cast.  So the original reporter finds this check always results in the
> function exiting early, and suggests the patch should be reverted.  What
> are your thoughts?

I remember adding that check for a reason; I ran into a case where it
was necessary.  Unfortunately either there wasn't a bug open for it or
I didn't reference it in the commit, and I don't remember the details.

Looking at the code later on, the check at the top certainly seems
wrong; we should be filling in the bridge info later if needed.  It
looks like Darren's commit came before mine and actually conflicts, but
we didn't catch it (I probably just did a blind rebase when I pushed my
change), so it's probably safe to just revert mine.

I'll do that now.

Thanks,
-- 
Jesse Barnes, Intel Open Source Technology Center