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

[Junio C Hamano <gitster@pobox.com>]

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);