git.vger.kernel.org archive mirror
 help / color / mirror / Atom feed
From: Jeff King <peff@peff.net>
To: git@vger.kernel.org
Subject: [PATCH 07/10] reset: free allocated tree buffers
Date: Tue, 5 Sep 2017 09:04:51 -0400	[thread overview]
Message-ID: <20170905130450.fsoglfwieei2ruir@sigill.intra.peff.net> (raw)
In-Reply-To: <20170905130149.agc3zp3s6i6e5aki@sigill.intra.peff.net>

We read the tree objects with fill_tree_descriptor(), but
never actually free the resulting buffers, causing a memory
leak. This isn't a huge deal because we call this code at
most twice per program invocation. But it does potentially
double our heap usage if you have large root trees. Let's
free the trees before returning.


Signed-off-by: Jeff King <peff@peff.net>
---
Actually, in the two-call case we repeat one of the trees,
so we _could_ in theory stash it away. But we don't do that
currently, and it's probably not worth the effort. My main
motivation is just shutting up the leak checker so I can
find actual important leaks.

 builtin/reset.c | 20 ++++++++++++++------
 1 file changed, 14 insertions(+), 6 deletions(-)

diff --git a/builtin/reset.c b/builtin/reset.c
index 34839db8ca..9e22b4a1dc 100644
--- a/builtin/reset.c
+++ b/builtin/reset.c
@@ -44,10 +44,11 @@ static inline int is_merge(void)
 
 static int reset_index(const struct object_id *oid, int reset_type, int quiet)
 {
-	int nr = 0;
+	int i, nr = 0;
 	struct tree_desc desc[2];
 	struct tree *tree;
 	struct unpack_trees_options opts;
+	int ret = -1;
 
 	memset(&opts, 0, sizeof(opts));
 	opts.head_idx = 1;
@@ -81,19 +82,26 @@ static int reset_index(const struct object_id *oid, int reset_type, int quiet)
 		opts.fn = twoway_merge;
 	}
 
-	if (!fill_tree_descriptor(desc + nr, oid))
-		return error(_("Failed to find tree of %s."), oid_to_hex(oid));
+	if (!fill_tree_descriptor(desc + nr, oid)) {
+		error(_("Failed to find tree of %s."), oid_to_hex(oid));
+		goto out;
+	}
 	nr++;
 
-	if (unpack_trees(nr, desc, &opts))
-		return -1;
+	if (unpack_trees(nr, desc, &opts)) 
+		goto out;
 
 	if (reset_type == MIXED || reset_type == HARD) {
 		tree = parse_tree_indirect(oid);
 		prime_cache_tree(&the_index, tree);
 	}
 
-	return 0;
+	ret = 0;
+
+out:
+	for (i = 0; i < nr; i++)
+		free((void *)desc[i].buffer);
+	return ret;
 }
 
 static void print_new_head_line(struct commit *commit)
-- 
2.14.1.721.gc5bc1565f1


  parent reply	other threads:[~2017-09-05 13:04 UTC|newest]

Thread overview: 42+ messages / expand[flat|nested]  mbox.gz  Atom feed  top
2017-09-05 13:01 [PATCH 0/10] towards clean leak-checker output Jeff King
2017-09-05 13:03 ` [PATCH 01/10] test-lib: --valgrind should not override --verbose-log Jeff King
2017-09-05 13:04 ` [PATCH 02/10] test-lib: set LSAN_OPTIONS to abort by default Jeff King
2017-09-05 13:04 ` [PATCH 03/10] add: free leaked pathspec after add_files_to_cache() Jeff King
2017-09-05 13:04 ` [PATCH 04/10] update-index: fix cache entry leak in add_one_file() Jeff King
2017-09-05 13:04 ` [PATCH 05/10] config: plug user_config leak Jeff King
2017-09-05 13:04 ` [PATCH 06/10] reset: make tree counting less confusing Jeff King
2017-09-05 13:04 ` Jeff King [this message]
2017-09-05 13:04 ` [PATCH 08/10] repository: free fields before overwriting them Jeff King
2017-09-05 13:05 ` [PATCH 09/10] set_git_dir: handle feeding gitdir to itself Jeff King
2017-09-07 19:06   ` Brandon Williams
2017-09-05 13:05 ` [PATCH 10/10] add UNLEAK annotation for reducing leak false positives Jeff King
2017-09-05 22:05   ` Stefan Beller
2017-09-07  9:17     ` Jeff King
2017-09-07 20:38       ` Stefan Beller
2017-09-12 14:34     ` Kaartic Sivaraam
2017-09-12 15:05       ` Jeff King
2017-09-13  7:13         ` Kaartic Sivaraam
2017-09-06 17:16   ` Martin Ågren
2017-09-07  9:00     ` Jeff King
2017-09-12 13:41   ` Kaartic Sivaraam
2017-09-12 15:29     ` Jeff King
2017-09-13  6:44       ` Kaartic Sivaraam
2017-09-05 17:50 ` [PATCH 0/10] towards clean leak-checker output Martin Ågren
2017-09-05 19:02   ` Jeff King
2017-09-05 20:41     ` Martin Ågren
2017-09-06 12:39       ` Jeff King
2017-09-06  1:42     ` Junio C Hamano
2017-09-06 12:28       ` [PATCH 0/2] simplifying !RUNTIME_PREFIX Jeff King
2017-09-06 12:30         ` [PATCH 1/2] system_path: move RUNTIME_PREFIX to a sub-function Jeff King
2017-09-06 13:23           ` Johannes Schindelin
2017-09-06 13:27             ` Jeff King
2017-09-06 12:32         ` [PATCH 2/2] git_extract_argv0_path: do nothing without RUNTIME_PREFIX Jeff King
2017-09-08  6:38 ` [PATCH v2 10/10] add UNLEAK annotation for reducing leak false positives Jeff King
2017-09-19 20:45   ` Jonathan Tan
2017-09-19 21:03     ` Jeff King
2017-09-19 21:34       ` [PATCH for jk/leak-checkers] git-compat-util: make UNLEAK less error-prone Jonathan Tan
2017-09-19 21:46         ` Jeff King
2017-09-19 22:10           ` [PATCH for jk/leak-checkers v2] " Jonathan Tan
2017-09-20  1:45       ` [PATCH v2 10/10] add UNLEAK annotation for reducing leak false positives Junio C Hamano
2017-09-20  2:28         ` Jeff King
2017-09-20  5:12           ` Junio C Hamano

Reply instructions:

You may reply publicly to this message via plain-text email
using any one of the following methods:

* Save the following mbox file, import it into your mail client,
  and reply-to-all from there: mbox

  Avoid top-posting and favor interleaved quoting:
  https://en.wikipedia.org/wiki/Posting_style#Interleaved_style

* Reply using the --to, --cc, and --in-reply-to
  switches of git-send-email(1):

  git send-email \
    --in-reply-to=20170905130450.fsoglfwieei2ruir@sigill.intra.peff.net \
    --to=peff@peff.net \
    --cc=git@vger.kernel.org \
    /path/to/YOUR_REPLY

  https://kernel.org/pub/software/scm/git/docs/git-send-email.html

* If your mail client supports setting the In-Reply-To header
  via mailto: links, try the mailto: link
Be sure your reply has a Subject: header at the top and a blank line before the message body.
This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox;
as well as URLs for NNTP newsgroup(s).