Re: [PATCH 1/4] list-objects-filter: treat NULL filter_options as "disabled"
- From
Junio C Hamano <gitster@pobox.com>
- Date
- May 5, 2020, 05:07 UTC
- Message-ID
- <xmqqh7wvc6l1.fsf@gitster.c.googlers.com>
- In-Reply-To
- <bb7d99ee2582d60e2413df56cb2c3fb06c18de8e.1588633810.git.me@ttaylorr.com>
Taylor Blau <me@ttaylorr.com> writes:
Show 20 quoted lines
> 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.
Show 21 quoted lines
>
> 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);