Re: [PATCH v12 6/7] eal: add unit tests for atomic bit access functions

["Mattias Rönnblom" <hofors@lysator.liu.se>]

On 2024-10-10 14:14, David Marchand wrote:
> On Thu, Oct 10, 2024 at 1:56 PM Mattias Rönnblom <hofors@lysator.liu.se> wrote:
>> OK. Nothing obvious from what I can see in the code. Unrelated: why did
>> you remove all empty lines in the "template" macros? Makes them much
>> harder to read.
> 
> Those macros are hard to read.
> 

Even the more reason to make them easier to read.

> There was an extra indent that resulted in splitting many lines.

They were just formatted with the same indentation as all other macros, 
no? So not "extra".

That said, the indentation leaves less room on for the actual code, so I 
have no issue with the removal of the standard multi-line macro 
indentation, although it wouldn't have hurt to have pointed this out 
earlier, so we wouldn't have to have this discussion after-the-fact.

> So I started with removing this extra indent.
> 
> Then, the trailing tabs (with some pseudo randomly mixed in spaces)
> were replaced with a single space before the \ for line continuation.
> This enhances code review for update: with a single systematic space,
> there is no need to consider if you must update all the context when
> adding a line longer than what was already present.

DPDK uses both the kernel-style multi-line macro formatting, and what 
you describe above.

Your argument above makes sense, but I also find the kernel style more 
visually appealing.

> 
> In the end, I was left with cases like:
>          <some code> \
>   \
>          <some code> \
> 
> And I preferred to remove this extra line as it did not enhance the
> clarity of those macros.
> 
> 

           <some code> \
                       \
           <some code> \

Would have been visually more appealing, but less consistent.

To me, removing these delimiting empty lines is no different from doing 
the same in a regular C function. Empty lines are delimiters, there to 
group related statements, and they serve a purpose. Now, the macros look 
like they are written by someone who doesn't care about readability.