git@vger.kernel.org mailing list mirror (one of many)
 help / color / mirror / code / Atom feed
From: Matthew DeVore <matvore@google.com>
To: git@vger.kernel.org
Cc: matvore@google.com, jonathantanmy@google.com,
	jeffhost@microsoft.com, gitster@pobox.com, pclouds@gmail.com
Subject: [PATCH] list-objects-filter: correct usage of ALLOC_GROW
Date: Fri, 31 May 2019 11:46:06 -0700	[thread overview]
Message-ID: <20190531184606.GA29730@comcast.net> (raw)

In the sparse filter data, array_frame array is used in a way such that
nr is the index of the last element. Fix this so that nr is actually the
number of elements in the array.

The filter_sparse_free function also has an unaddressed TODO to free the
memory associated with the sparse filter data. Address that TODO and fix
the memory leak.

Signed-off-by: Matthew DeVore <matvore@google.com>
---
 list-objects-filter.c | 21 ++++++++++++---------
 1 file changed, 12 insertions(+), 9 deletions(-)

diff --git a/list-objects-filter.c b/list-objects-filter.c
index ee449de3f7..19158cc712 100644
--- a/list-objects-filter.c
+++ b/list-objects-filter.c
@@ -348,28 +348,30 @@ static enum list_objects_filter_result filter_sparse(
 	switch (filter_situation) {
 	default:
 		BUG("unknown filter_situation: %d", filter_situation);
 
 	case LOFS_BEGIN_TREE:
 		assert(obj->type == OBJ_TREE);
 		dtype = DT_DIR;
 		val = is_excluded_from_list(pathname, strlen(pathname),
 					    filename, &dtype, &filter_data->el,
 					    r->index);
-		if (val < 0)
-			val = filter_data->array_frame[filter_data->nr].defval;
+		if (val < 0) {
+			val = filter_data->array_frame[filter_data->nr - 1]
+				.defval;
+		}
 
 		ALLOC_GROW(filter_data->array_frame, filter_data->nr + 1,
 			   filter_data->alloc);
-		filter_data->nr++;
 		filter_data->array_frame[filter_data->nr].defval = val;
 		filter_data->array_frame[filter_data->nr].child_prov_omit = 0;
+		filter_data->nr++;
 
 		/*
 		 * A directory with this tree OID may appear in multiple
 		 * places in the tree. (Think of a directory move or copy,
 		 * with no other changes, so the OID is the same, but the
 		 * full pathnames of objects within this directory are new
 		 * and may match is_excluded() patterns differently.)
 		 * So we cannot mark this directory as SEEN (yet), since
 		 * that will prevent process_tree() from revisiting this
 		 * tree object with other pathname prefixes.
@@ -380,46 +382,45 @@ static enum list_objects_filter_result filter_sparse(
 		 * We always show all tree objects.  A future optimization
 		 * may want to attempt to narrow this.
 		 */
 		if (obj->flags & FILTER_SHOWN_BUT_REVISIT)
 			return LOFR_ZERO;
 		obj->flags |= FILTER_SHOWN_BUT_REVISIT;
 		return LOFR_DO_SHOW;
 
 	case LOFS_END_TREE:
 		assert(obj->type == OBJ_TREE);
-		assert(filter_data->nr > 0);
+		assert(filter_data->nr > 1);
 
-		frame = &filter_data->array_frame[filter_data->nr];
-		filter_data->nr--;
+		frame = &filter_data->array_frame[--filter_data->nr];
 
 		/*
 		 * Tell our parent directory if any of our children were
 		 * provisionally omitted.
 		 */
-		filter_data->array_frame[filter_data->nr].child_prov_omit |=
+		filter_data->array_frame[filter_data->nr - 1].child_prov_omit |=
 			frame->child_prov_omit;
 
 		/*
 		 * If there are NO provisionally omitted child objects (ALL child
 		 * objects in this folder were INCLUDED), then we can mark the
 		 * folder as SEEN (so we will not have to revisit it again).
 		 */
 		if (!frame->child_prov_omit)
 			return LOFR_MARK_SEEN;
 		return LOFR_ZERO;
 
 	case LOFS_BLOB:
 		assert(obj->type == OBJ_BLOB);
 		assert((obj->flags & SEEN) == 0);
 
-		frame = &filter_data->array_frame[filter_data->nr];
+		frame = &filter_data->array_frame[filter_data->nr - 1];
 
 		dtype = DT_REG;
 		val = is_excluded_from_list(pathname, strlen(pathname),
 					    filename, &dtype, &filter_data->el,
 					    r->index);
 		if (val < 0)
 			val = frame->defval;
 		if (val > 0) {
 			if (filter_data->omits)
 				oidset_remove(filter_data->omits, &obj->oid);
@@ -446,39 +447,40 @@ static enum list_objects_filter_result filter_sparse(
 		 */
 		frame->child_prov_omit = 1;
 		return LOFR_ZERO;
 	}
 }
 
 
 static void filter_sparse_free(void *filter_data)
 {
 	struct filter_sparse_data *d = filter_data;
-	/* TODO free contents of 'd' */
+	free(d->array_frame);
 	free(d);
 }
 
 static void *filter_sparse_oid__init(
 	struct oidset *omitted,
 	struct list_objects_filter_options *filter_options,
 	filter_object_fn *filter_fn,
 	filter_free_fn *filter_free_fn)
 {
 	struct filter_sparse_data *d = xcalloc(1, sizeof(*d));
 	d->omits = omitted;
 	if (add_excludes_from_blob_to_list(filter_options->sparse_oid_value,
 					   NULL, 0, &d->el) < 0)
 		die("could not load filter specification");
 
 	ALLOC_GROW(d->array_frame, d->nr + 1, d->alloc);
 	d->array_frame[d->nr].defval = 0; /* default to include */
 	d->array_frame[d->nr].child_prov_omit = 0;
+	d->nr++;
 
 	*filter_fn = filter_sparse;
 	*filter_free_fn = filter_sparse_free;
 	return d;
 }
 
 static void *filter_sparse_path__init(
 	struct oidset *omitted,
 	struct list_objects_filter_options *filter_options,
 	filter_object_fn *filter_fn,
@@ -486,20 +488,21 @@ static void *filter_sparse_path__init(
 {
 	struct filter_sparse_data *d = xcalloc(1, sizeof(*d));
 	d->omits = omitted;
 	if (add_excludes_from_file_to_list(filter_options->sparse_path_value,
 					   NULL, 0, &d->el, NULL) < 0)
 		die("could not load filter specification");
 
 	ALLOC_GROW(d->array_frame, d->nr + 1, d->alloc);
 	d->array_frame[d->nr].defval = 0; /* default to include */
 	d->array_frame[d->nr].child_prov_omit = 0;
+	d->nr++;
 
 	*filter_fn = filter_sparse;
 	*filter_free_fn = filter_sparse_free;
 	return d;
 }
 
 typedef void *(*filter_init_fn)(
 	struct oidset *omitted,
 	struct list_objects_filter_options *filter_options,
 	filter_object_fn *filter_fn,
-- 
2.17.1


             reply	other threads:[~2019-05-31 18:46 UTC|newest]

Thread overview: 2+ messages / expand[flat|nested]  mbox.gz  Atom feed  top
2019-05-31 18:46 Matthew DeVore [this message]
2019-05-31 19:35 ` [PATCH] list-objects-filter: correct usage of ALLOC_GROW Jeff Hostetler

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

  List information: http://vger.kernel.org/majordomo-info.html

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

  git send-email \
    --in-reply-to=20190531184606.GA29730@comcast.net \
    --to=matvore@google.com \
    --cc=git@vger.kernel.org \
    --cc=gitster@pobox.com \
    --cc=jeffhost@microsoft.com \
    --cc=jonathantanmy@google.com \
    --cc=pclouds@gmail.com \
    /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.
Code repositories for project(s) associated with this public inbox

	https://80x24.org/mirrors/git.git

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 read-only IMAP folder(s) and NNTP newsgroup(s).