All of lore.kernel.org
 help / color / mirror / Atom feed
From: Jeff King <peff@peff.net>
To: git@vger.kernel.org
Subject: [PATCH 3/3] traverse_trees(): use stack array for name entries
Date: Thu, 30 Jan 2020 04:53:38 -0500	[thread overview]
Message-ID: <20200130095338.GC840531@coredump.intra.peff.net> (raw)
In-Reply-To: <20200130095155.GA839988@coredump.intra.peff.net>

We heap-allocate our arrays of name_entry structs, etc, with one entry
per tree we're asked to traverse. The code does a raw multiplication in
the xmalloc() call, which I find when auditing for integer overflows
during allocation.

We could "fix" this by using ALLOC_ARRAY() instead. But as it turns out,
the maximum size of these arrays is limited at compile time:

  - merge_trees() always passes in 3 trees

  - unpack_trees() and its brethren never pass in more than
    MAX_UNPACK_TREES

So we can simplify even further by just using a stack array and bounding
it with MAX_UNPACK_TREES. There should be no concern with overflowing
the stack, since MAX_UNPACK_TREES is only 8 and the structs themselves
are small.

Note that since we're replacing xcalloc(), we have to move one of the
NULL initializations into a loop.

Signed-off-by: Jeff King <peff@peff.net>
---

This does increase the coupling between tree-walk and unpack-trees a
bit. I'd be OK just switching to ALLOC_ARRAY(), too. I doubt the
performance improvement is measurable, and the cleanup free() calls are
already there.

 tree-walk.c | 13 ++++++++-----
 1 file changed, 8 insertions(+), 5 deletions(-)

diff --git a/tree-walk.c b/tree-walk.c
index d5a8e096a6..3093cf7098 100644
--- a/tree-walk.c
+++ b/tree-walk.c
@@ -410,15 +410,20 @@ int traverse_trees(struct index_state *istate,
 		   struct traverse_info *info)
 {
 	int error = 0;
-	struct name_entry *entry = xmalloc(n*sizeof(*entry));
+	struct name_entry entry[MAX_UNPACK_TREES];
 	int i;
-	struct tree_desc_x *tx = xcalloc(n, sizeof(*tx));
+	struct tree_desc_x tx[ARRAY_SIZE(entry)];
 	struct strbuf base = STRBUF_INIT;
 	int interesting = 1;
 	char *traverse_path;
 
-	for (i = 0; i < n; i++)
+	if (n >= ARRAY_SIZE(entry))
+		BUG("traverse_trees() called with too many trees (%d)", n);
+
+	for (i = 0; i < n; i++) {
 		tx[i].d = t[i];
+		tx[i].skip = NULL;
+	}
 
 	if (info->prev) {
 		strbuf_make_traverse_path(&base, info->prev,
@@ -506,10 +511,8 @@ int traverse_trees(struct index_state *istate,
 			if (mask & (1ul << i))
 				update_extended_entry(tx + i, entry + i);
 	}
-	free(entry);
 	for (i = 0; i < n; i++)
 		free_extended_entry(tx + i);
-	free(tx);
 	free(traverse_path);
 	info->traverse_path = NULL;
 	strbuf_release(&base);
-- 
2.25.0.515.gaba5347bc6

  parent reply	other threads:[~2020-01-30  9:53 UTC|newest]

Thread overview: 13+ messages / expand[flat|nested]  mbox.gz  Atom feed  top
2020-01-30  9:51 [PATCH 0/3] some minor memory allocation cleanups Jeff King
2020-01-30  9:52 ` [PATCH 1/3] normalize_path_copy(): document "dst" size expectations Jeff King
2020-01-30 20:12   ` Taylor Blau
2020-01-31  8:45     ` Jeff King
2020-01-30  9:52 ` [PATCH 2/3] walker_fetch(): avoid raw array length computation Jeff King
2020-01-30  9:53 ` Jeff King [this message]
2020-01-30 14:57   ` [PATCH 3/3] traverse_trees(): use stack array for name entries Elijah Newren
2020-01-31  9:29     ` Jeff King
2020-01-31 18:52       ` Elijah Newren
2020-02-01 11:39         ` [PATCH] tree-walk.c: break circular dependency with unpack-trees Jeff King
2020-02-01 15:32           ` Elijah Newren
2020-01-30 14:59 ` [PATCH 0/3] some minor memory allocation cleanups Elijah Newren
2020-01-30 23:03 ` Taylor Blau

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=20200130095338.GC840531@coredump.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 an external index of several public inboxes,
see mirroring instructions on how to clone and mirror
all data and code used by this external index.