diff mbox series

[1/4] list-objects-filter: treat NULL filter_options as "disabled"

Message ID bb7d99ee2582d60e2413df56cb2c3fb06c18de8e.1588633810.git.me@ttaylorr.com (mailing list archive)
State New, archived
Headers show
Series pack-bitmap: use bitmaps for traversals with '--filter=tree:0' | expand

Commit Message

Taylor Blau May 4, 2020, 11:12 p.m. UTC
From: Jeff King <peff@peff.net>

In most callers, we have an actual list_objects_filter_options struct,
and if no filtering is desired its "choice" element will be
LOFC_DISABLED. However, some code may have only a pointer to such a
struct which may be NULL (because _their_ callers didn't care about
filtering, either). Rather than forcing them to handle this explicitly
like:

  if (filter_options)
          traverse_commit_list_filtered(filter_options, revs,
	                                show_commit, show_object,
					show_data, NULL);
  else
          traverse_commit_list(revs, show_commit, show_object,
	                             show_data);

let's just treat a NULL filter_options the same as LOFC_DISABLED. We
only need a small change, since that option struct is converted into a
real filter only in the "init" function.

Signed-off-by: Jeff King <peff@peff.net>
Signed-off-by: Taylor Blau <me@ttaylorr.com>
---
 list-objects-filter.c | 3 +++
 1 file changed, 3 insertions(+)

Comments

Junio C Hamano May 5, 2020, 5:07 a.m. UTC | #1
Taylor Blau <me@ttaylorr.com> writes:

> From: Jeff King <peff@peff.net>
>
> In most callers, we have an actual list_objects_filter_options struct,
> and if no filtering is desired its "choice" element will be
> LOFC_DISABLED. However, some code may have only a pointer to such a
> struct which may be NULL (because _their_ callers didn't care about
> filtering, either). Rather than forcing them to handle this explicitly
> like:
>
>   if (filter_options)
>           traverse_commit_list_filtered(filter_options, revs,
> 	                                show_commit, show_object,
> 					show_data, NULL);
>   else
>           traverse_commit_list(revs, show_commit, show_object,
> 	                             show_data);
>
> let's just treat a NULL filter_options the same as LOFC_DISABLED. We
> only need a small change, since that option struct is converted into a
> real filter only in the "init" function.

This is not a problem the patch introduces, but anytime we improve
the revision traversal API, Documentation/MyFirstObjectWalk.txt
risks to be left behind (in the sense that it still is correct but
is now suboptimal) or outright broken (when we change the function
signature).  I wonder if we can do some "mark-up" to that tutorial
so that we can extract runnable code in some of the tests and make
sure the code snippet in the documentation is still OK.

As to the patch, there apparently is no existing caller that passes
NULL to the function (or they would be immediately segfaulting), so
by definition, this step alone cannot break anything, but at the
same time, it is a bit hard to assess if this is a good change with
this step alone.  So let's read on.

Thanks.

>
> Signed-off-by: Jeff King <peff@peff.net>
> Signed-off-by: Taylor Blau <me@ttaylorr.com>
> ---
>  list-objects-filter.c | 3 +++
>  1 file changed, 3 insertions(+)
>
> diff --git a/list-objects-filter.c b/list-objects-filter.c
> index 1e8d4e763d..0a3ef3cab3 100644
> --- a/list-objects-filter.c
> +++ b/list-objects-filter.c
> @@ -663,6 +663,9 @@ struct filter *list_objects_filter__init(
>  
>  	assert((sizeof(s_filters) / sizeof(s_filters[0])) == LOFC__COUNT);
>  
> +	if (!filter_options)
> +		return NULL;
> +
>  	if (filter_options->choice >= LOFC__COUNT)
>  		BUG("invalid list-objects filter choice: %d",
>  		    filter_options->choice);
diff mbox series

Patch

diff --git a/list-objects-filter.c b/list-objects-filter.c
index 1e8d4e763d..0a3ef3cab3 100644
--- a/list-objects-filter.c
+++ b/list-objects-filter.c
@@ -663,6 +663,9 @@  struct filter *list_objects_filter__init(
 
 	assert((sizeof(s_filters) / sizeof(s_filters[0])) == LOFC__COUNT);
 
+	if (!filter_options)
+		return NULL;
+
 	if (filter_options->choice >= LOFC__COUNT)
 		BUG("invalid list-objects filter choice: %d",
 		    filter_options->choice);