[PATCH] raw/ifpga: fix comma warnings

[PATCH] raw/ifpga: fix comma warnings

[Stephen Hemminger]

The driver was open coding TAILQ_FOREACH_SAFE() in a manner
that triggered warnings. Replace it with the standard one
from bsd queue.h.

Signed-off-by: Stephen Hemminger <stephen@networkplumber.org>
---
 drivers/raw/ifpga/base/ifpga_enumerate.c | 4 +---
 drivers/raw/ifpga/base/opae_hw_api.h     | 7 +++++++
 2 files changed, 8 insertions(+), 3 deletions(-)

diff --git a/drivers/raw/ifpga/base/ifpga_enumerate.c b/drivers/raw/ifpga/base/ifpga_enumerate.c
index 61eb6601ea..085fb6db40 100644
--- a/drivers/raw/ifpga/base/ifpga_enumerate.c
+++ b/drivers/raw/ifpga/base/ifpga_enumerate.c
@@ -725,9 +725,7 @@ static void dfl_fpga_enum_info_free(struct dfl_fpga_enum_info *info)
 		return;
 
 	/* remove all device feature lists in the list. */
-	for (dfl = TAILQ_FIRST(&info->dfls);
-		dfl && (tmp = TAILQ_NEXT(dfl, node), 1);
-		dfl = tmp) {
+	TAILQ_FOREACH_SAFE(dfl, &info->dfls, node, tmp) {
 		TAILQ_REMOVE(&info->dfls, dfl, node);
 		opae_free(dfl);
 	}
diff --git a/drivers/raw/ifpga/base/opae_hw_api.h b/drivers/raw/ifpga/base/opae_hw_api.h
index 57750022dd..63cb616731 100644
--- a/drivers/raw/ifpga/base/opae_hw_api.h
+++ b/drivers/raw/ifpga/base/opae_hw_api.h
@@ -10,6 +10,13 @@
 #include <stdio.h>
 #include <sys/queue.h>
 
+#ifndef TAILQ_FOREACH_SAFE
+#define	TAILQ_FOREACH_SAFE(var, head, field, tvar)			\
+	for ((var) = TAILQ_FIRST((head));				\
+	    (var) && ((tvar) = TAILQ_NEXT((var), field), 1);		\
+	    (var) = (tvar))
+#endif
+
 #include "opae_osdep.h"
 #include "opae_intel_max10.h"
 #include "opae_eth_group.h"
-- 
2.51.0

Re: [PATCH] raw/ifpga: fix comma warnings

[Bruce Richardson]

On Wed, Mar 11, 2026 at 05:23:36PM -0700, Stephen Hemminger wrote:
> The driver was open coding TAILQ_FOREACH_SAFE() in a manner
> that triggered warnings. Replace it with the standard one
> from bsd queue.h.
> 
> Signed-off-by: Stephen Hemminger <stephen@networkplumber.org>
> ---
>  drivers/raw/ifpga/base/ifpga_enumerate.c | 4 +---
>  drivers/raw/ifpga/base/opae_hw_api.h     | 7 +++++++
>  2 files changed, 8 insertions(+), 3 deletions(-)
> 
> diff --git a/drivers/raw/ifpga/base/ifpga_enumerate.c b/drivers/raw/ifpga/base/ifpga_enumerate.c
> index 61eb6601ea..085fb6db40 100644
> --- a/drivers/raw/ifpga/base/ifpga_enumerate.c
> +++ b/drivers/raw/ifpga/base/ifpga_enumerate.c
> @@ -725,9 +725,7 @@ static void dfl_fpga_enum_info_free(struct dfl_fpga_enum_info *info)
>  		return;
>  
>  	/* remove all device feature lists in the list. */
> -	for (dfl = TAILQ_FIRST(&info->dfls);
> -		dfl && (tmp = TAILQ_NEXT(dfl, node), 1);
> -		dfl = tmp) {
> +	TAILQ_FOREACH_SAFE(dfl, &info->dfls, node, tmp) {
>  		TAILQ_REMOVE(&info->dfls, dfl, node);
>  		opae_free(dfl);
>  	}
> diff --git a/drivers/raw/ifpga/base/opae_hw_api.h b/drivers/raw/ifpga/base/opae_hw_api.h
> index 57750022dd..63cb616731 100644
> --- a/drivers/raw/ifpga/base/opae_hw_api.h
> +++ b/drivers/raw/ifpga/base/opae_hw_api.h
> @@ -10,6 +10,13 @@
>  #include <stdio.h>
>  #include <sys/queue.h>
>  
> +#ifndef TAILQ_FOREACH_SAFE
> +#define	TAILQ_FOREACH_SAFE(var, head, field, tvar)			\
> +	for ((var) = TAILQ_FIRST((head));				\
> +	    (var) && ((tvar) = TAILQ_NEXT((var), field), 1);		\
> +	    (var) = (tvar))
> +#endif
> +

I am curious as to how this is not causing warnings but the original code
is. Have we got builds where we are triggering this macro definition, or is
ever build tested already got the define?

Re: [PATCH] raw/ifpga: fix comma warnings

[Stephen Hemminger]

On Thu, 12 Mar 2026 09:15:26 +0000
Bruce Richardson <bruce.richardson@intel.com> wrote:

> On Wed, Mar 11, 2026 at 05:23:36PM -0700, Stephen Hemminger wrote:
> > The driver was open coding TAILQ_FOREACH_SAFE() in a manner
> > that triggered warnings. Replace it with the standard one
> > from bsd queue.h.
> > 
> > Signed-off-by: Stephen Hemminger <stephen@networkplumber.org>
> > ---
> >  drivers/raw/ifpga/base/ifpga_enumerate.c | 4 +---
> >  drivers/raw/ifpga/base/opae_hw_api.h     | 7 +++++++
> >  2 files changed, 8 insertions(+), 3 deletions(-)
> > 
> > diff --git a/drivers/raw/ifpga/base/ifpga_enumerate.c b/drivers/raw/ifpga/base/ifpga_enumerate.c
> > index 61eb6601ea..085fb6db40 100644
> > --- a/drivers/raw/ifpga/base/ifpga_enumerate.c
> > +++ b/drivers/raw/ifpga/base/ifpga_enumerate.c
> > @@ -725,9 +725,7 @@ static void dfl_fpga_enum_info_free(struct dfl_fpga_enum_info *info)
> >  		return;
> >  
> >  	/* remove all device feature lists in the list. */
> > -	for (dfl = TAILQ_FIRST(&info->dfls);
> > -		dfl && (tmp = TAILQ_NEXT(dfl, node), 1);
> > -		dfl = tmp) {
> > +	TAILQ_FOREACH_SAFE(dfl, &info->dfls, node, tmp) {
> >  		TAILQ_REMOVE(&info->dfls, dfl, node);
> >  		opae_free(dfl);
> >  	}
> > diff --git a/drivers/raw/ifpga/base/opae_hw_api.h b/drivers/raw/ifpga/base/opae_hw_api.h
> > index 57750022dd..63cb616731 100644
> > --- a/drivers/raw/ifpga/base/opae_hw_api.h
> > +++ b/drivers/raw/ifpga/base/opae_hw_api.h
> > @@ -10,6 +10,13 @@
> >  #include <stdio.h>
> >  #include <sys/queue.h>
> >  
> > +#ifndef TAILQ_FOREACH_SAFE
> > +#define	TAILQ_FOREACH_SAFE(var, head, field, tvar)			\
> > +	for ((var) = TAILQ_FIRST((head));				\
> > +	    (var) && ((tvar) = TAILQ_NEXT((var), field), 1);		\
> > +	    (var) = (tvar))
> > +#endif
> > +  
> 
> I am curious as to how this is not causing warnings but the original code
> is. Have we got builds where we are triggering this macro definition, or is
> ever build tested already got the define?

You have to ask to enable comma warnings, I think it is currently disabled
at the driver level.

Re: [PATCH] raw/ifpga: fix comma warnings

[Bruce Richardson]

On Thu, Mar 12, 2026 at 09:03:03AM -0700, Stephen Hemminger wrote:
> On Thu, 12 Mar 2026 09:15:26 +0000
> Bruce Richardson <bruce.richardson@intel.com> wrote:
> 
> > On Wed, Mar 11, 2026 at 05:23:36PM -0700, Stephen Hemminger wrote:
> > > The driver was open coding TAILQ_FOREACH_SAFE() in a manner
> > > that triggered warnings. Replace it with the standard one
> > > from bsd queue.h.
> > > 
> > > Signed-off-by: Stephen Hemminger <stephen@networkplumber.org>
> > > ---
> > >  drivers/raw/ifpga/base/ifpga_enumerate.c | 4 +---
> > >  drivers/raw/ifpga/base/opae_hw_api.h     | 7 +++++++
> > >  2 files changed, 8 insertions(+), 3 deletions(-)
> > > 
> > > diff --git a/drivers/raw/ifpga/base/ifpga_enumerate.c b/drivers/raw/ifpga/base/ifpga_enumerate.c
> > > index 61eb6601ea..085fb6db40 100644
> > > --- a/drivers/raw/ifpga/base/ifpga_enumerate.c
> > > +++ b/drivers/raw/ifpga/base/ifpga_enumerate.c
> > > @@ -725,9 +725,7 @@ static void dfl_fpga_enum_info_free(struct dfl_fpga_enum_info *info)
> > >  		return;
> > >  
> > >  	/* remove all device feature lists in the list. */
> > > -	for (dfl = TAILQ_FIRST(&info->dfls);
> > > -		dfl && (tmp = TAILQ_NEXT(dfl, node), 1);
> > > -		dfl = tmp) {
> > > +	TAILQ_FOREACH_SAFE(dfl, &info->dfls, node, tmp) {
> > >  		TAILQ_REMOVE(&info->dfls, dfl, node);
> > >  		opae_free(dfl);
> > >  	}
> > > diff --git a/drivers/raw/ifpga/base/opae_hw_api.h b/drivers/raw/ifpga/base/opae_hw_api.h
> > > index 57750022dd..63cb616731 100644
> > > --- a/drivers/raw/ifpga/base/opae_hw_api.h
> > > +++ b/drivers/raw/ifpga/base/opae_hw_api.h
> > > @@ -10,6 +10,13 @@
> > >  #include <stdio.h>
> > >  #include <sys/queue.h>
> > >  
> > > +#ifndef TAILQ_FOREACH_SAFE
> > > +#define	TAILQ_FOREACH_SAFE(var, head, field, tvar)			\
> > > +	for ((var) = TAILQ_FIRST((head));				\
> > > +	    (var) && ((tvar) = TAILQ_NEXT((var), field), 1);		\
> > > +	    (var) = (tvar))
> > > +#endif
> > > +  
> > 
> > I am curious as to how this is not causing warnings but the original code
> > is. Have we got builds where we are triggering this macro definition, or is
> > ever build tested already got the define?
> 
> You have to ask to enable comma warnings, I think it is currently disabled
> at the driver level.

Yes, it's disabled for the drivers, but if we enable it for raw/ifpga
driver, how come this macro doesn't give the warning when it uses the exact
same structure as the original code?

Re: [PATCH] raw/ifpga: fix comma warnings

[Stephen Hemminger]

On Thu, 12 Mar 2026 16:28:23 +0000
Bruce Richardson <bruce.richardson@intel.com> wrote:

> Yes, it's disabled for the drivers, but if we enable it for raw/ifpga
> driver, how come this macro doesn't give the warning when it uses the exact
> same structure as the original code?

Short answer, clang treats macros as special case for comma operator.

As always AI has longer answer...


Great question — this is a well-known Clang behavior that trips people up.

Clang's `-Wcomma` warning fires when it sees a comma operator where the left-hand side result is discarded, since that's often a bug (someone meant `&&` or `;` instead of `,`). The expression `(tmp = TAILQ_NEXT(dfl, node), 1)` is a textbook trigger: the assignment result is discarded and only the `1` is used.

However, Clang deliberately suppresses most warnings — including `-Wcomma` — for code that originates from macro expansions. The rationale is that macro-expanded code is an established idiom the user didn't write directly at that call site, so warning on it would generate noise that the caller can't reasonably fix. The warning logic checks whether the expression's source location traces back to a macro expansion and, if so, skips the diagnostic.

So the two cases are semantically identical after preprocessing, but Clang's diagnostic engine distinguishes them by source location:

**Inline version** — the comma operator appears directly in your source code → Clang assumes you wrote it intentionally and warns you in case you didn't.

**Macro version** — the comma operator is inside `TAILQ_FOREACH_SAFE`'s expansion → Clang sees the macro origin, treats it as an intentional idiom, and suppresses the warning.

If you want to silence it in the inline case without rewriting the loop, you have a few options: cast the left side to `void` explicitly — `((void)(tmp = TAILQ_NEXT(dfl, node)), 1)` — which signals to Clang that the discard is intentional, or use a pragma to locally disable `-Wcomma`, or just use the macro (which is the idiomatic approach for TAILQ iteration anyway).