On Sun, Jun 2, 2013 at 10:46 AM, René Scharfe
<rene.scharfe@xxxxxxxxxxxxxx> wrote:
> The merge functions duplicate entries as needed and they don't free
> them. Release them in unpack_nondirectories, the same function
> where they were allocated, after we're done.
>
> As suggested by Felipe, use the same loop style (zero-based for loop)
> for freeing as for allocating.
>
> Improved-by: Felipe Contreras <felipe.contreras@xxxxxxxxx>
> Signed-off-by: René Scharfe <rene.scharfe@xxxxxxxxxxxxxx>
> ---
> unpack-trees.c | 13 ++++++++++---
> 1 file changed, 10 insertions(+), 3 deletions(-)
>
> diff --git a/unpack-trees.c b/unpack-trees.c
> index 2dbc05d..57b4074 100644
> --- a/unpack-trees.c
> +++ b/unpack-trees.c
> @@ -600,9 +600,16 @@ static int unpack_nondirectories(int n, unsigned long mask,
> src[i + o->merge] = create_ce_entry(info, names + i, stage);
> }
>
> - if (o->merge)
> - return call_unpack_fn((const struct cache_entry * const *)src,
> - o);
> + if (o->merge) {
> + int rc = call_unpack_fn((const struct cache_entry * const *)src,
> + o);
> + for (i = 0; i < n; i++) {
> + struct cache_entry *ce = src[i + o->merge];
> + if (ce != o->df_conflict_entry)
It's possible that ce is NULL, but you didn't add that check because
free(NULL) still works? Or because ce cannot be NULL?
If it's the former, I think it's clearer if we check that ce is not
NULL either way.
Otherwise it's OK by me.
> + free(ce);
> + }
> + return rc;
> + }
--
Felipe Contreras
--
To unsubscribe from this list: send the line "unsubscribe git" in
the body of a message to majordomo@xxxxxxxxxxxxxxx
More majordomo info at http://vger.kernel.org/majordomo-info.html