All of lore.kernel.org
 help / color / mirror / 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

* 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.
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.