From: Elijah Newren <newren@gmail.com>
Date: Sat, 22 Apr 2023 02:04:41 GMT
Subject: Re: [PATCH] merge-ort: fix calling merge_finalize() with no intermediate merge
Message-ID: <CABPp-BGVEWM_N3eXjfEGjemeNzrLMmqDOuWKCvzOuHGeMueoJQ@mail.gmail.com>
In-Reply-To: <d5c93b5b-32b9-ec50-1661-06f73fa37f5f@github.com>

On Thu, Apr 20, 2023 at 6:10 AM Derrick Stolee <derrickstolee@github.com> wrote:
>
> On 4/20/2023 3:14 AM, Elijah Newren via GitGitGadget wrote:
> > From: Elijah Newren <newren@gmail.com>
>
> > 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.