unpack-trees: plug minor memory leak

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>

René Scharfe committed Mar 6, 2012 at 20:37 UTC 6ff264ee05cc8fd6b2c796623eadb8662444a458
1 file changed +16 -9
unpack-trees.c
+16 -9
@@ -102,21 +102,28 @@ void setup_unpack_trees_porcelain(struct unpack_trees_options *opts,
102 opts->unpack_rejects[i].strdup_strings = 1;
103 }
104
105 -static void add_entry(struct unpack_trees_options *o, struct cache_entry *ce,
106 - unsigned int set, unsigned int clear)
105 +static void do_add_entry(struct unpack_trees_options *o, struct cache_entry *ce,
106 + unsigned int set, unsigned int clear)
107 {
108 - unsigned int size = ce_size(ce);
109 - struct cache_entry *new = xmalloc(size);
110 -
108 clear |= CE_HASHED | CE_UNHASHED;
109
110 if (set & CE_REMOVE)
111 set |= CE_WT_REMOVE;
112
113 + ce->next = NULL;
114 + ce->ce_flags = (ce->ce_flags & ~clear) | set;
115 + add_index_entry(&o->result, ce,
116 + ADD_CACHE_OK_TO_ADD | ADD_CACHE_OK_TO_REPLACE);
117 +}
118 +
119 +static void add_entry(struct unpack_trees_options *o, struct cache_entry *ce,
120 + unsigned int set, unsigned int clear)
121 +{
122 + unsigned int size = ce_size(ce);
123 + struct cache_entry *new = xmalloc(size);
124 +
125 memcpy(new, ce, size);
117 - new->next = NULL;
118 - new->ce_flags = (new->ce_flags & ~clear) | set;
119 - add_index_entry(&o->result, new, ADD_CACHE_OK_TO_ADD|ADD_CACHE_OK_TO_REPLACE);
126 + do_add_entry(o, new, set, clear);
127 }
128
129 /*
@@ -587,7 +594,7 @@ static int unpack_nondirectories(int n, unsigned long mask,
594
595 for (i = 0; i < n; i++)
596 if (src[i] && src[i] != o->df_conflict_entry)
590 - add_entry(o, src[i], 0, 0);
597 + do_add_entry(o, src[i], 0, 0);
598 return 0;
599 }
600