Hi Peff
It's great to see you back on the list
On 12/07/2022 08:19, Jeff King wrote:
On Mon, Jul 11, 2022 at 11:00:06AM +0100, Phillip Wood wrote:
But you've also changed every single callsite anyway.
So why not just change:
if (XDL_ALLOC_GROW(recs, ...))
To:
XDL_ALLOC_GROW(recs, ...);
if (!recs)
Because I think it's ugly, we'd never define a function as void(int* result,
args...) and I don't think we should do that for a macro where we can avoid
it. The existing ALLOC_* macros make sense as statements because they die on
failure.
Those macros are already unlike functions, though. They take a variable
_name_, not a pointer to a variable, and assign to it. A version which
looked like a function would have to be:
if (XDL_ALLOC_GROW(&recs, ...))
So in my head I read them as assignment statements already, making the
expression-like form a bit jarring.
I see what you're saying, I agree it is not exactly analogous. I tend to
think of assignment as an expression because it returns the value that's
being assigned, though here it's a bit muddy because the assignment is
hidden inside the macro and there is no tell-tale '&' as there would be
if it were calling function.
Just my two cents. I don't care _too_ much either way, but I'd probably
be swayed if one implementation is much simpler than the other.
I don't think there is much difference in the complexity in this case.
In general I think that using a function with a macro wrapper can make
the implementation more readable as the function it is not littered with
the extra parentheses and backslashes that the same code in a macro
requires.
Best Wishes
Phillip
