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.
-Peff
