Re: [PATCH v4 24/27] builtin/rebase: do not assign default backend to non-constant field
- From
Patrick Steinhardt <ps@pks.im>
- Date
- Jun 6, 2024, 09:50 UTC
- Message-ID
- <ZmGGWdClbLxNfRwF@tanuki>
- In-Reply-To
- <d83f640e-2528-4326-bc46-d2b99f83a153@gmail.com>
On Wed, Jun 05, 2024 at 02:06:55PM +0100, Phillip Wood wrote:
Show 28 quoted lines
> 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