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
lmpx.com only provides a reader for public news (NNTP) servers. It is not affiliated with the servers or forums shown here and is not responsible for the content of articles, which is written by their respective authors.