@matheusbsbs / matheus-bagliano / commits / 9430147ca0

list-objects-filter: encapsulate filter components

Encapsulate filter_fn, filter_free_fn, and filter_data into their own opaque struct. Due to opaqueness, filter_fn and filter_free_fn can no longer be accessed directly by users. Currently, all usages of filter_fn are guarded by a necessary check: (obj->flags & NOT_USER_GIVEN) && filter_fn Take the opportunity to include this check into the new function list_objects_filter__filter_object(), so that we no longer need to write this check at every caller of the filter function. Also, the init functions in list-objects-filter.c no longer need to confusingly return the filter constituents in various places (filter_fn and filter_free_fn as out parameters, and filter_data as the function's return value); they can just initialize the "struct filter" passed in. Helped-by: Jeff Hostetler <git@jeffhostetler.com> Helped-by: Jonathan Tan <jonathantanmy@google.com> Helped-by: Junio C Hamano <gitster@pobox.com> Signed-off-by: Matthew DeVore <matvore@google.com> Signed-off-by: Junio C Hamano <gitster@pobox.com>

Matthew DeVore committed Jun 27, 2019 at 15:54 UTC 9430147ca0aab0189d7e52df97b95a0985fc0c8a
3 files changed +113 -89
list-objects-filter.c
+75 -37
@@ -26,6 +26,20 @@
26 */
27 #define FILTER_SHOWN_BUT_REVISIT (1<<21)
28
29 +struct filter {
30 + enum list_objects_filter_result (*filter_object_fn)(
31 + struct repository *r,
32 + enum list_objects_filter_situation filter_situation,
33 + struct object *obj,
34 + const char *pathname,
35 + const char *filename,
36 + void *filter_data);
37 +
38 + void (*free_fn)(void *filter_data);
39 +
40 + void *filter_data;
41 +};
42 +
43 /*
44 * A filter for list-objects to omit ALL blobs from the traversal.
45 * And to OPTIONALLY collect a list of the omitted OIDs.
@@ -67,18 +81,17 @@ static enum list_objects_filter_result filter_blobs_none(
81 }
82 }
83
70 -static void *filter_blobs_none__init(
84 +static void filter_blobs_none__init(
85 struct oidset *omitted,
86 struct list_objects_filter_options *filter_options,
73 - filter_object_fn *filter_fn,
74 - filter_free_fn *filter_free_fn)
87 + struct filter *filter)
88 {
89 struct filter_blobs_none_data *d = xcalloc(1, sizeof(*d));
90 d->omits = omitted;
91
79 - *filter_fn = filter_blobs_none;
80 - *filter_free_fn = free;
81 - return d;
92 + filter->filter_data = d;
93 + filter->filter_object_fn = filter_blobs_none;
94 + filter->free_fn = free;
95 }
96
97 /*
@@ -201,11 +214,10 @@ static void filter_trees_free(void *filter_data) {
214 free(d);
215 }
216
204 -static void *filter_trees_depth__init(
217 +static void filter_trees_depth__init(
218 struct oidset *omitted,
219 struct list_objects_filter_options *filter_options,
207 - filter_object_fn *filter_fn,
208 - filter_free_fn *filter_free_fn)
220 + struct filter *filter)
221 {
222 struct filter_trees_depth_data *d = xcalloc(1, sizeof(*d));
223 d->omits = omitted;
@@ -213,9 +225,9 @@ static void *filter_trees_depth__init(
225 d->exclude_depth = filter_options->tree_exclude_depth;
226 d->current_depth = 0;
227
216 - *filter_fn = filter_trees_depth;
217 - *filter_free_fn = filter_trees_free;
218 - return d;
228 + filter->filter_data = d;
229 + filter->filter_object_fn = filter_trees_depth;
230 + filter->free_fn = filter_trees_free;
231 }
232
233 /*
@@ -281,19 +293,18 @@ include_it:
293 return LOFR_MARK_SEEN | LOFR_DO_SHOW;
294 }
295
284 -static void *filter_blobs_limit__init(
296 +static void filter_blobs_limit__init(
297 struct oidset *omitted,
298 struct list_objects_filter_options *filter_options,
287 - filter_object_fn *filter_fn,
288 - filter_free_fn *filter_free_fn)
299 + struct filter *filter)
300 {
301 struct filter_blobs_limit_data *d = xcalloc(1, sizeof(*d));
302 d->omits = omitted;
303 d->max_bytes = filter_options->blob_limit_value;
304
294 - *filter_fn = filter_blobs_limit;
295 - *filter_free_fn = free;
296 - return d;
305 + filter->filter_data = d;
306 + filter->filter_object_fn = filter_blobs_limit;
307 + filter->free_fn = free;
308 }
309
310 /*
@@ -456,11 +467,10 @@ static void filter_sparse_free(void *filter_data)
467 free(d);
468 }
469
459 -static void *filter_sparse_oid__init(
470 +static void filter_sparse_oid__init(
471 struct oidset *omitted,
472 struct list_objects_filter_options *filter_options,
462 - filter_object_fn *filter_fn,
463 - filter_free_fn *filter_free_fn)
473 + struct filter *filter)
474 {
475 struct filter_sparse_data *d = xcalloc(1, sizeof(*d));
476 d->omits = omitted;
@@ -473,16 +483,15 @@ static void *filter_sparse_oid__init(
483 d->array_frame[d->nr].child_prov_omit = 0;
484 d->nr++;
485
476 - *filter_fn = filter_sparse;
477 - *filter_free_fn = filter_sparse_free;
478 - return d;
486 + filter->filter_data = d;
487 + filter->filter_object_fn = filter_sparse;
488 + filter->free_fn = filter_sparse_free;
489 }
490
481 -typedef void *(*filter_init_fn)(
491 +typedef void (*filter_init_fn)(
492 struct oidset *omitted,
493 struct list_objects_filter_options *filter_options,
484 - filter_object_fn *filter_fn,
485 - filter_free_fn *filter_free_fn);
494 + struct filter *filter);
495
496 /*
497 * Must match "enum list_objects_filter_choice".
@@ -495,12 +504,11 @@ static filter_init_fn s_filters[] = {
504 filter_sparse_oid__init,
505 };
506
498 -void *list_objects_filter__init(
507 +struct filter *list_objects_filter__init(
508 struct oidset *omitted,
500 - struct list_objects_filter_options *filter_options,
501 - filter_object_fn *filter_fn,
502 - filter_free_fn *filter_free_fn)
509 + struct list_objects_filter_options *filter_options)
510 {
511 + struct filter *filter;
512 filter_init_fn init_fn;
513
514 assert((sizeof(s_filters) / sizeof(s_filters[0])) == LOFC__COUNT);
@@ -510,10 +518,40 @@ void *list_objects_filter__init(
518 filter_options->choice);
519
520 init_fn = s_filters[filter_options->choice];
513 - if (init_fn)
514 - return init_fn(omitted, filter_options,
515 - filter_fn, filter_free_fn);
516 - *filter_fn = NULL;
517 - *filter_free_fn = NULL;
518 - return NULL;
521 + if (!init_fn)
522 + return NULL;
523 +
524 + filter = xcalloc(1, sizeof(*filter));
525 + init_fn(omitted, filter_options, filter);
526 + return filter;
527 +}
528 +
529 +enum list_objects_filter_result list_objects_filter__filter_object(
530 + struct repository *r,
531 + enum list_objects_filter_situation filter_situation,
532 + struct object *obj,
533 + const char *pathname,
534 + const char *filename,
535 + struct filter *filter)
536 +{
537 + if (filter && (obj->flags & NOT_USER_GIVEN))
538 + return filter->filter_object_fn(r, filter_situation, obj,
539 + pathname, filename,
540 + filter->filter_data);
541 + /*
542 + * No filter is active or user gave object explicitly. In this case,
543 + * always show the object (except when LOFS_END_TREE, since this tree
544 + * had already been shown when LOFS_BEGIN_TREE).
545 + */
546 + if (filter_situation == LOFS_END_TREE)
547 + return 0;
548 + return LOFR_MARK_SEEN | LOFR_DO_SHOW;
549 +}
550 +
551 +void list_objects_filter__free(struct filter *filter)
552 +{
553 + if (!filter)
554 + return;
555 + filter->free_fn(filter->filter_data);
556 + free(filter);
557 }
list-objects-filter.h
+16 -19
@@ -60,30 +60,27 @@ enum list_objects_filter_situation {
60 LOFS_BLOB
61 };
62
63 -typedef enum list_objects_filter_result (*filter_object_fn)(
63 +struct filter;
64 +
65 +/* Constructor for the set of defined list-objects filters. */
66 +struct filter *list_objects_filter__init(
67 + struct oidset *omitted,
68 + struct list_objects_filter_options *filter_options);
69 +
70 +/*
71 + * Lets `filter` decide how to handle the `obj`. If `filter` is NULL, this
72 + * function behaves as expected if no filter is configured: all objects are
73 + * included.
74 + */
75 +enum list_objects_filter_result list_objects_filter__filter_object(
76 struct repository *r,
77 enum list_objects_filter_situation filter_situation,
78 struct object *obj,
79 const char *pathname,
80 const char *filename,
69 - void *filter_data);
70 -
71 -typedef void (*filter_free_fn)(void *filter_data);
81 + struct filter *filter);
82
73 -/*
74 - * Constructor for the set of defined list-objects filters.
75 - * Returns a generic "void *filter_data".
76 - *
77 - * The returned "filter_fn" will be used by traverse_commit_list()
78 - * to filter the results.
79 - *
80 - * The returned "filter_free_fn" is a destructor for the
81 - * filter_data.
82 - */
83 -void *list_objects_filter__init(
84 - struct oidset *omitted,
85 - struct list_objects_filter_options *filter_options,
86 - filter_object_fn *filter_fn,
87 - filter_free_fn *filter_free_fn);
83 +/* Destroys `filter`. Does nothing if `filter` is null. */
84 +void list_objects_filter__free(struct filter *filter);
85
86 #endif /* LIST_OBJECTS_FILTER_H */
list-objects.c
+22 -33
@@ -18,8 +18,7 @@ struct traversal_context {
18 show_object_fn show_object;
19 show_commit_fn show_commit;
20 void *show_data;
21 - filter_object_fn filter_fn;
22 - void *filter_data;
21 + struct filter *filter;
22 };
23
24 static void process_blob(struct traversal_context *ctx,
@@ -29,7 +28,7 @@ static void process_blob(struct traversal_context *ctx,
28 {
29 struct object *obj = &blob->object;
30 size_t pathlen;
32 - enum list_objects_filter_result r = LOFR_MARK_SEEN | LOFR_DO_SHOW;
31 + enum list_objects_filter_result r;
32
33 if (!ctx->revs->blob_objects)
34 return;
@@ -54,11 +53,10 @@ static void process_blob(struct traversal_context *ctx,
53
54 pathlen = path->len;
55 strbuf_addstr(path, name);
57 - if ((obj->flags & NOT_USER_GIVEN) && ctx->filter_fn)
58 - r = ctx->filter_fn(ctx->revs->repo,
59 - LOFS_BLOB, obj,
60 - path->buf, &path->buf[pathlen],
61 - ctx->filter_data);
56 + r = list_objects_filter__filter_object(ctx->revs->repo,
57 + LOFS_BLOB, obj,
58 + path->buf, &path->buf[pathlen],
59 + ctx->filter);
60 if (r & LOFR_MARK_SEEN)
61 obj->flags |= SEEN;
62 if (r & LOFR_DO_SHOW)
@@ -157,7 +155,7 @@ static void process_tree(struct traversal_context *ctx,
155 struct object *obj = &tree->object;
156 struct rev_info *revs = ctx->revs;
157 int baselen = base->len;
160 - enum list_objects_filter_result r = LOFR_MARK_SEEN | LOFR_DO_SHOW;
158 + enum list_objects_filter_result r;
159 int failed_parse;
160
161 if (!revs->tree_objects)
@@ -186,11 +184,10 @@ static void process_tree(struct traversal_context *ctx,
184 }
185
186 strbuf_addstr(base, name);
189 - if ((obj->flags & NOT_USER_GIVEN) && ctx->filter_fn)
190 - r = ctx->filter_fn(ctx->revs->repo,
191 - LOFS_BEGIN_TREE, obj,
192 - base->buf, &base->buf[baselen],
193 - ctx->filter_data);
187 + r = list_objects_filter__filter_object(ctx->revs->repo,
188 + LOFS_BEGIN_TREE, obj,
189 + base->buf, &base->buf[baselen],
190 + ctx->filter);
191 if (r & LOFR_MARK_SEEN)
192 obj->flags |= SEEN;
193 if (r & LOFR_DO_SHOW)
@@ -203,16 +200,14 @@ static void process_tree(struct traversal_context *ctx,
200 else if (!failed_parse)
201 process_tree_contents(ctx, tree, base);
202
206 - if ((obj->flags & NOT_USER_GIVEN) && ctx->filter_fn) {
207 - r = ctx->filter_fn(ctx->revs->repo,
208 - LOFS_END_TREE, obj,
209 - base->buf, &base->buf[baselen],
210 - ctx->filter_data);
211 - if (r & LOFR_MARK_SEEN)
212 - obj->flags |= SEEN;
213 - if (r & LOFR_DO_SHOW)
214 - ctx->show_object(obj, base->buf, ctx->show_data);
215 - }
203 + r = list_objects_filter__filter_object(ctx->revs->repo,
204 + LOFS_END_TREE, obj,
205 + base->buf, &base->buf[baselen],
206 + ctx->filter);
207 + if (r & LOFR_MARK_SEEN)
208 + obj->flags |= SEEN;
209 + if (r & LOFR_DO_SHOW)
210 + ctx->show_object(obj, base->buf, ctx->show_data);
211
212 strbuf_setlen(base, baselen);
213 free_tree_buffer(tree);
@@ -402,8 +397,7 @@ void traverse_commit_list(struct rev_info *revs,
397 ctx.show_commit = show_commit;
398 ctx.show_object = show_object;
399 ctx.show_data = show_data;
405 - ctx.filter_fn = NULL;
406 - ctx.filter_data = NULL;
400 + ctx.filter = NULL;
401 do_traverse(&ctx);
402 }
403
@@ -416,17 +410,12 @@ void traverse_commit_list_filtered(
410 struct oidset *omitted)
411 {
412 struct traversal_context ctx;
419 - filter_free_fn filter_free_fn = NULL;
413
414 ctx.revs = revs;
415 ctx.show_object = show_object;
416 ctx.show_commit = show_commit;
417 ctx.show_data = show_data;
425 - ctx.filter_fn = NULL;
426 -
427 - ctx.filter_data = list_objects_filter__init(omitted, filter_options,
428 - &ctx.filter_fn, &filter_free_fn);
418 + ctx.filter = list_objects_filter__init(omitted, filter_options);
419 do_traverse(&ctx);
430 - if (ctx.filter_data && filter_free_fn)
431 - filter_free_fn(ctx.filter_data);
420 + list_objects_filter__free(ctx.filter);
421 }