On Tue, Jun 04, 2024 at 03:06:38PM +0100, Phillip Wood wrote:
> Hi Patrick
>
> On 04/06/2024 13:38, Patrick Steinhardt wrote:
> > The `struct rebase_options::default_backend` field is a non-constant
> > string, but is being assigned a constant via `REBASE_OPTIONS_INIT`.
> > Refactor the code to initialize and release options via two functions
> > `rebase_options_init()` and `rebase_options_release()`. Like this, we
> > can easily adapt the former funnction to use `xstrdup()` on the default
> > value without hiding it away in a macro.
>
> Personally I'd be happy with
>
> - .default_backend = "merge", \
> + .default_backend = xstrdup("merge"), \
>
> rather than adding an init function. I do agree that adding
> rebase_options_release() is a good idea and the rest of the changes look
> good to me
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.
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.
Patrick
Attachment:
signature.asc
Description: PGP signature
