Re: [PATCH 1/3] merge-recursive: fix memory leak

From: Jeff King <peff@peff.net>
Date: Aug 29, 2017, 08:12 UTC
Message-ID: <20170829081203.uwt6yfve5uatbhch@sigill.intra.peff.net>
In-Reply-To: <20170828202829.3056-2-kewillf@microsoft.com>

On Mon, Aug 28, 2017 at 02:28:27PM -0600, Kevin Willford wrote:

Show 6 quoted lines

> In merge_trees if process_renames or process_entry returns less
> than zero, the method will just return and not free re_merge,
> re_head, or entries.
> 
> This change cleans up the allocated variables before returning
> to the caller.

Good catch. I suspect this function could stand to be refactored a bit. For instance, pulling those inner bits of the conditional into a helper would let us do:

  re_merge = get_renames(...);
  ... other setup ...

  clean = our_new_helper(re_merge, ...);

  string_clear(re_merge);
  ... other cleanup ...

  if (clean < 0)
	return clean;

without having to resort to a goto. But certainly I don't mind this much more minimal change, which fixes the actual functionality problem.

-Peff