list-objects-filter: correct usage of ALLOC_GROW
PATCH 1/1 list-objects-filter: correct usage of ALLOC_GROW
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 <[email protected]>
--- list-objects-filter.c | 21 ++++++++++++--------- 1 file changed, 12 insertions(+), 9 deletions(-)
list-objects-filter.c +12 -9
@@ -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,
Signature · 1 line
2.17.1
Quoted 3 lines · Matthew DeVore
In the sparse filter data,
array_framearray 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.
I don't remember why I did it that way.
Thanks for fixing!
Jeff