* Martin Mares <mj@xxxxxx>:
> Hello!
>
> > We've been exposing slot information in /sys/bus/pci/slots for a
> > long time now (as long as a hotplug driver or slot detection
> > driver like pci_slot is loaded).
>
> This is much better than the previous version, however you are still
> leaking memory.
Hm, this hunk didn't get it?
@@ -296,6 +357,7 @@ static void sysfs_cleanup_dev(struct pci_dev *d)
close(a->fd);
a->fd = -1;
}
+ pci_mfree(d->phy_slot);
}
> (Also, please follow the indentation style of the original code.)
I thought I did, but I'll double-check.
> > +static int
> > +sysfs_fill_info(struct pci_dev *d, int flags)
> > +{
> > + int ret;
> > + static int firstime;
> > +
> > + ret = pci_generic_fill_info(d, flags);
> > + if (!firstime && flags & PCI_FILL_PHYS_SLOT) {
> > + sysfs_fill_slots(d);
> > + firstime = 1;
> > + }
> > +
> > + return ret;
> > +}
>
> Using a static variable for this is wrong, since it breaks applications
> working with multiple pci_access structures.
I was thinking of having sysfs_fill_slots() set the
PCI_FILL_PHYS_SLOT bit in d->known_fields, but that is on a
per-device basis, right?
Wouldn't that lead to us checking over and over for devices that
do not have slot entries in sysfs?
Finally, did this version address the ABI issue? I wasn't sure
about that.
Thanks.
/ac
--
To unsubscribe from this list: send the line "unsubscribe linux-pci" in
the body of a message to majordomo@xxxxxxxxxxxxxxx
More majordomo info at http://vger.kernel.org/majordomo-info.html
