On Thu, Apr 20, 2023 at 6:10 AM Derrick Stolee <derrickstolee@xxxxxxxxxx> wrote:
>
> On 4/20/2023 3:14 AM, Elijah Newren via GitGitGadget wrote:
> > From: Elijah Newren <newren@xxxxxxxxx>
>
> > While at it, ensure the FREE_AND_NULL() in the function does something
> > useful with the nulling aspect, namely sets result->priv to NULL rather
> > than a mere temporary.
>
> Good call. It also makes the code look better.
>
> > void merge_finalize(struct merge_options *opt,
> > struct merge_result *result)
> > {
> > - struct merge_options_internal *opti = result->priv;
> > -
> > if (opt->renormalize)
> > git_attr_set_direction(GIT_ATTR_CHECKIN);
> > assert(opt->priv == NULL);
> >
> > - clear_or_reinit_internal_opts(opti, 0);
> > - FREE_AND_NULL(opti);
> > + if (!result->priv)
> > + return;
> > + clear_or_reinit_internal_opts(result->priv, 0);
> > + FREE_AND_NULL(result->priv);
>
> Perhaps this would be better as
>
> if (result->priv) {
> clear_or_reinit_internal_opts(result->priv, 0);
> FREE_AND_NULL(result->priv);
> }
>
> to avoid an accidental addition of code to the end of this
> method that doesn't depend on result->priv?
Ooh, yes, that'd be a good improvement; thanks.
