Page MenuHomeFreeBSD

pf: Workaround set but unused warning.
ClosedPublic

Authored by jhb on Apr 7 2022, 7:43 PM.
Tags
None
Referenced Files
Unknown Object (File)
Fri, May 17, 6:24 PM
Unknown Object (File)
May 14 2024, 10:30 AM
Unknown Object (File)
May 14 2024, 8:02 AM
Unknown Object (File)
May 14 2024, 6:10 AM
Unknown Object (File)
Feb 12 2024, 3:03 PM
Unknown Object (File)
Jan 14 2024, 8:23 AM
Unknown Object (File)
Nov 16 2023, 7:54 PM
Unknown Object (File)
Nov 14 2023, 7:46 PM

Details

Summary

The RB_NEXT macro does not use its middle argument since commit
5fce408cc44c737267aaaf0dcecd3454ba9089cd in 2004 (which ironically
fixed an "unused parameter" warning by introducing this warning in all
consumers). RB_PREV has also copied this unfortunate behavior of an
unused argument.

This results in 'parent' not being used. To workaround, inline the
value of 'parent' as the second argument to RB_NEXT.

Diff Detail

Repository
rG FreeBSD src repository
Lint
Lint Not Applicable
Unit
Tests Not Applicable

Event Timeline

jhb requested review of this revision.Apr 7 2022, 7:43 PM

That works, but I wonder if we shouldn't just update the macro to remove the middle argument. There are only about 100 consumers of RB_NEXT in the tree and maybe 40 of RB_PREV.

This revision is now accepted and ready to land.Apr 8 2022, 6:55 AM
In D34833#789523, @kp wrote:

... I wonder if we shouldn't just update the macro to remove the middle argument. There are only about 100 consumers of RB_NEXT in the tree and maybe 40 of RB_PREV.

That'd look a bit like this: D34842

In D34833#789572, @kp wrote:
In D34833#789523, @kp wrote:

... I wonder if we shouldn't just update the macro to remove the middle argument. There are only about 100 consumers of RB_NEXT in the tree and maybe 40 of RB_PREV.

That'd look a bit like this: D34842

Thinking about this again, it's probably not the right way to go because there may be out-of-tree (userspace) consumers of RB_NEXT/RB_PREV, and they'd break.

In D34833#789576, @kp wrote:
In D34833#789572, @kp wrote:
In D34833#789523, @kp wrote:

... I wonder if we shouldn't just update the macro to remove the middle argument. There are only about 100 consumers of RB_NEXT in the tree and maybe 40 of RB_PREV.

That'd look a bit like this: D34842

Thinking about this again, it's probably not the right way to go because there may be out-of-tree (userspace) consumers of RB_NEXT/RB_PREV, and they'd break.

Yes, it is out of tree consumers I'd worry about. @arichardson once pointed me at some evil hacks one can do to support a variable number of arguments to a macro so that we could have both a 2 argument and 3 argument version of RB_NEXT/RB_PREV. If we could add that, we could then fix the ones in pf to drop the unused argument which would be nice. (I would have other uses for it as well if that can be done non-terribly).

In D34833#789639, @jhb wrote:

Yes, it is out of tree consumers I'd worry about. @arichardson once pointed me at some evil hacks one can do to support a variable number of arguments to a macro so that we could have both a 2 argument and 3 argument version of RB_NEXT/RB_PREV. If we could add that, we could then fix the ones in pf to drop the unused argument which would be nice. (I would have other uses for it as well if that can be done non-terribly).

Something like the answers in this:

https://stackoverflow.com/questions/11761703/overloading-macro-on-number-of-arguments

After further mulling, I think I will just stick with this workaround for now vs messing with RB_NEXT/PREV.

This revision was automatically updated to reflect the committed changes.