> From: Stephen Hemminger [mailto:step...@networkplumber.org]
> Sent: Tuesday, 16 January 2024 23.50
>
> On Tue, 16 Jan 2024 23:14:36 +0100
> Morten Brørup <m...@smartsharesystems.com> wrote:
>
> > > +1 for #2 just make it a block.
> >
> > I prefer that you implement the workaround in the RTE_BUILD_BUG_ON()
> macro, by surrounding it by "do { } while (0)", like this:
> >
> > #define RTE_BUILD_BUG_ON(condition) do { static_assert(!(condition),
> #condition); } while (0)
> >
> > And please mention the workaround reason, with the reference to the
> clang bug, in the source code comment describing the function.
> >
> > The godbolt link in the clang issue seems happy with this workaround.
>
> In the patch series sent, already did that. Didn't need to do { }
> while(0) hack to make it work.
It works in the problematic file, but not generally.
Without the do/while, "RTE_BUILD_BUG_ON(condition);" will expand to two lines:
{ static_assert(...); }
;
... which may be problematic when used with if/else without curly braces.
However, I do admit that this problem is probably purely theoretical.
> Reference is in commit message.
Yes, but if someone only looks at the source code, the workaround looks strange
and unnecessary.
Personally, I prefer not having to dig into commit logs when reviewing code.
Maybe it's just me. I'll leave the decision to you.
