Re: [PATCH v8 0/7] Add pci_dev_for_each_resource() helper and update users
Bjorn Helgaas <[email protected]>
| Newsgroups | gmane.linux.acpi.devel,gmane.linux.kernel.pci,gmane.linux.kernel,gmane.linux.ports.sh.devel,gmane.linux.ports.sparc,gmane.comp.emulators.xen.devel,gmane.linux.ports.arm.kernel,gmane.linux.ports.ppc64.devel,gmane.linux.ports.mips,gmane.linux.ports.alpha |
|---|---|
| Message-ID | <ZHe8dKb3f392MfBO@bhelgaas> |
On Wed, May 31, 2023 at 08:48:35PM +0200, Jonas Gorski wrote: > ... > Looking at the code I understand where coverity is coming from: > > #define __pci_dev_for_each_res0(dev, res, ...) \ > for (unsigned int __b = 0; \ > res = pci_resource_n(dev, __b), __b < PCI_NUM_RESOURCES; \ > __b++) > > res will be assigned before __b is checked for being less than > PCI_NUM_RESOURCES, making it point to behind the array at the end of > the last loop iteration. > > Rewriting the test expression as > > __b < PCI_NUM_RESOURCES && (res = pci_resource_n(dev, __b)); > > should avoid the (coverity) warning by making use of lazy evaluation. > > It probably makes the code slightly less performant as res will now be > checked for being not NULL (which will always be true), but I doubt it > will be significant (or in any hot paths). Thanks a lot for looking into this! I think you're right, and I think the rewritten expression is more logical as well. Do you want to post a patch for it? Bjorn