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

From: Patrick Steinhardt <[email-masked]>
Date: Thu, 6 Jun 2024 11:50:17 +0200

On Wed, Jun 05, 2024 at 02:06:55PM +0100, Phillip Wood wrote:
> 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).

Okay, given that there is precedent, and given that Junio also seems to
slightly lean into the direction of the `_INIT` macro, I'll adapt this
as proposed by you.

Patrick