Skip to content

Commit

Permalink
unpack-trees: plug minor memory leak
Browse files Browse the repository at this point in the history
The allocations made by unpack_nondirectories() using create_ce_entry()
are never freed.

In the non-merge case, we duplicate them using add_entry() and later
only look at the first allocated element (src[0]), perhaps even only
by mistake.  Split out the actual addition from add_entry() into the
new helper do_add_entry() and call this non-duplicating function
instead of add_entry() to avoid the leak.

Valgrind reports this for the command "git archive v1.7.9" without
the patch:

  ==13372== LEAK SUMMARY:
  ==13372==    definitely lost: 230,986 bytes in 2,325 blocks
  ==13372==    indirectly lost: 0 bytes in 0 blocks
  ==13372==      possibly lost: 98 bytes in 1 blocks
  ==13372==    still reachable: 2,259,198 bytes in 3,243 blocks
  ==13372==         suppressed: 0 bytes in 0 blocks

And with the patch applied:

  ==13375== LEAK SUMMARY:
  ==13375==    definitely lost: 65 bytes in 1 blocks
  ==13375==    indirectly lost: 0 bytes in 0 blocks
  ==13375==      possibly lost: 0 bytes in 0 blocks
  ==13375==    still reachable: 2,364,417 bytes in 3,245 blocks
  ==13375==         suppressed: 0 bytes in 0 blocks

Signed-off-by: Rene Scharfe <rene.scharfe@lsrfire.ath.cx>
Signed-off-by: Junio C Hamano <gitster@pobox.com>
  • Loading branch information
René Scharfe authored and Junio C Hamano committed Apr 10, 2012
1 parent 97e5954 commit 6ff264e
Showing 1 changed file with 16 additions and 9 deletions.
25 changes: 16 additions & 9 deletions unpack-trees.c
Original file line number Diff line number Diff line change
Expand Up @@ -102,21 +102,28 @@ void setup_unpack_trees_porcelain(struct unpack_trees_options *opts,
opts->unpack_rejects[i].strdup_strings = 1;
}

static void add_entry(struct unpack_trees_options *o, struct cache_entry *ce,
unsigned int set, unsigned int clear)
static void do_add_entry(struct unpack_trees_options *o, struct cache_entry *ce,
unsigned int set, unsigned int clear)
{
unsigned int size = ce_size(ce);
struct cache_entry *new = xmalloc(size);

clear |= CE_HASHED | CE_UNHASHED;

if (set & CE_REMOVE)
set |= CE_WT_REMOVE;

ce->next = NULL;
ce->ce_flags = (ce->ce_flags & ~clear) | set;
add_index_entry(&o->result, ce,
ADD_CACHE_OK_TO_ADD | ADD_CACHE_OK_TO_REPLACE);
}

static void add_entry(struct unpack_trees_options *o, struct cache_entry *ce,
unsigned int set, unsigned int clear)
{
unsigned int size = ce_size(ce);
struct cache_entry *new = xmalloc(size);

memcpy(new, ce, size);
new->next = NULL;
new->ce_flags = (new->ce_flags & ~clear) | set;
add_index_entry(&o->result, new, ADD_CACHE_OK_TO_ADD|ADD_CACHE_OK_TO_REPLACE);
do_add_entry(o, new, set, clear);
}

/*
Expand Down Expand Up @@ -587,7 +594,7 @@ static int unpack_nondirectories(int n, unsigned long mask,

for (i = 0; i < n; i++)
if (src[i] && src[i] != o->df_conflict_entry)
add_entry(o, src[i], 0, 0);
do_add_entry(o, src[i], 0, 0);
return 0;
}

Expand Down

0 comments on commit 6ff264e

Please sign in to comment.