Re: [PATCH v4 24/27] builtin/rebase: do not assign default backend to non-constant field

From: Phillip Wood <phillip.wood123@gmail.com>
Date: Jun 5, 2024, 13:06 UTC
Message-ID: <d83f640e-2528-4326-bc46-d2b99f83a153@gmail.com>
In-Reply-To: <Zl_6anUl_C3W2j2Z@tanuki>

On 05/06/2024 06:40, Patrick Steinhardt wrote:

> On Tue, Jun 04, 2024 at 03:06:38PM +0100, Phillip Wood wrote:

> Do we have any other cases where we allocate inside of a `_INIT` style
> macro? If so, I'd go with that precedence and just allocate inside of
> the macro. But if we don't, then I think I'm leaning more towards the
> way I did it in this patch.

I recently added an allocation to REPLAY_OPTS_INIT, I'm not sure if there are any others. In general code that does

	struct foo = FOO_INIT;

and does not call

	foo_release();

is asking for trouble so I don't feel that allocations in _INIT macros are generally a problem. The only reason not to have allocations in an _INIT macro is if it might be used to initialize a file scope or global variable but that's not the case here.

> Happy to be convinced otherwise, I don't really feel all that strongly
> about this. I'm merely aiming for the interface wth least surprises.

I'm not that fussed either, but I do prefer our _INIT macros over _init() functions as I think they're nicer to use and easier to write (no need to worry about memset() to zero out the struct).

Best Wishes

Phillip