On Wed, May 31, 2023 at 08:48:35PM +0200, Jonas Gorski wrote:

On Tue, 30 May 2023 at 23:34, Bjorn Helgaas [off-list ref] wrote:

quoted

On Fri, May 12, 2023 at 02:48:51PM -0500, Bjorn Helgaas wrote:

quoted

On Fri, May 12, 2023 at 01:56:29PM +0300, Andy Shevchenko wrote:

quoted

On Tue, May 09, 2023 at 01:21:22PM -0500, Bjorn Helgaas wrote:

quoted

On Tue, Apr 04, 2023 at 11:11:01AM -0500, Bjorn Helgaas wrote:

quoted

On Thu, Mar 30, 2023 at 07:24:27PM +0300, Andy Shevchenko wrote:

quoted

Provide two new helper macros to iterate over PCI device resources and
convert users.

quoted

Applied 2-7 to pci/resource for v6.4, thanks, I really like this!

This is 09cc90063240 ("PCI: Introduce pci_dev_for_each_resource()")
upstream now.

Coverity complains about each use,

It needs more clarification here. Use of reduced variant of the
macro or all of them? If the former one, then I can speculate that
Coverity (famous for false positives) simply doesn't understand `for
(type var; var ...)` code.

True, Coverity finds false positives.  It flagged every use in
drivers/pci and drivers/pnp.  It didn't mention the arch/alpha, arm,
mips, powerpc, sh, or sparc uses, but I think it just didn't look at
those.

It flagged both:

  pbus_size_io    pci_dev_for_each_resource(dev, r)
  pbus_size_mem   pci_dev_for_each_resource(dev, r, i)

Here's a spreadsheet with a few more details (unfortunately I don't
know how to make it dump the actual line numbers or analysis like I
pasted below, so "pci_dev_for_each_resource" doesn't appear).  These
are mostly in the "Drivers-PCI" component.

https://docs.google.com/spreadsheets/d/1ohOJwxqXXoDUA0gwopgk-z-6ArLvhN7AZn4mIlDkHhQ/edit?usp=sharing

These particular reports are in the "High Impact Outstanding" tab.

Where are we at?  Are we going to ignore this because some Coverity
reports are false positives?

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.

Which is fine and you stumbled over the same mistake I made, that's why the
documentation has been added to describe why the heck this macro is written
the way it's written.

Coverity sucks.

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.

Obviously NAK.

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).

-- 
With Best Regards,
Andy Shevchenko