From 97e5954bdcf46c76e4a8147172ff7c5a4f633e0d Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Ren=C3=A9=20Scharfe?= Date: Tue, 10 Apr 2012 20:55:58 +0200 Subject: [PATCH 1/2] unpack-trees: don't perform any index operation if we're not merging src[0] points to the index entry in the merge case and to the first tree to unpack in the non-merge case. We only want to mark the index entry, so check first if we're merging. Signed-off-by: Rene Scharfe Signed-off-by: Junio C Hamano --- unpack-trees.c | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/unpack-trees.c b/unpack-trees.c index 7c9ecf665d..60b728e495 100644 --- a/unpack-trees.c +++ b/unpack-trees.c @@ -772,7 +772,7 @@ static int unpack_callback(int n, unsigned long mask, unsigned long dirmask, str if (unpack_nondirectories(n, mask, dirmask, src, names, info) < 0) return -1; - if (src[0]) { + if (o->merge && src[0]) { if (ce_stage(src[0])) mark_ce_used_same_name(src[0], o); else From 6ff264ee05cc8fd6b2c796623eadb8662444a458 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Ren=C3=A9=20Scharfe?= Date: Tue, 6 Mar 2012 20:37:23 +0100 Subject: [PATCH 2/2] 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 Signed-off-by: Junio C Hamano --- unpack-trees.c | 25 ++++++++++++++++--------- 1 file changed, 16 insertions(+), 9 deletions(-) diff --git a/unpack-trees.c b/unpack-trees.c index 60b728e495..36523da22a 100644 --- a/unpack-trees.c +++ b/unpack-trees.c @@ -102,21 +102,28 @@ void setup_unpack_trees_porcelain(struct unpack_trees_options *opts, opts->unpack_rejects[i].strdup_strings = 1; } +static void do_add_entry(struct unpack_trees_options *o, struct cache_entry *ce, + unsigned int set, unsigned int clear) +{ + 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); - clear |= CE_HASHED | CE_UNHASHED; - - if (set & CE_REMOVE) - set |= CE_WT_REMOVE; - 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); } /* @@ -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; }