On 11-jun-2023 23:21:06, Jeff King wrote:
> On Sun, Jun 11, 2023 at 08:49:43PM +0200, Rubén Justo wrote:
>
> > In e89f151db1 (branch: move --set-upstream-to behavior to
> > dwim_and_setup_tracking(), 2022-01-28) the string returned by
> > dwim_branch_start() was not freed, resulting in a memory leak.
>
> Yep, looks good. One small comment:
>
> > diff --git a/branch.c b/branch.c
> > index ba3914adf5..a7333a4c32 100644
> > --- a/branch.c
> > +++ b/branch.c
> > @@ -638,9 +638,10 @@ void dwim_and_setup_tracking(struct repository *r, const char *new_ref,
> > const char *orig_ref, enum branch_track track,
> > int quiet)
> > {
> > - char *real_orig_ref;
> > + char *real_orig_ref = NULL;
> > dwim_branch_start(r, orig_ref, track, &real_orig_ref, NULL);
> > setup_tracking(new_ref, real_orig_ref, track, quiet);
> > + free(real_orig_ref);
> > }
>
> Adding the NULL-initialization signals that we expect that real_orig_ref
> might sometimes _not_ be filled in by dwim_branch_start(). But I think
> it always will be; and if it weren't, that would make passing it to
> setup_tracking() rather suspicious.
>
> So I don't think the NULL initialization is wrong, and certainly it is
> more defensive. But I find it a little misleading.
I don't have any objection to this. But I've seen Junio has already
merged this to 'next'. I'm not going to re-roll this commit. Sorry.
Thank you for your review.
