On Fri, Oct 18, 2024 at 02:04:18PM +0200, Toon Claes wrote:
> Patrick Steinhardt <ps@xxxxxx> writes:
> > diff --git a/list-objects-filter-options.c b/list-objects-filter-options.c
> > index 00611107d20..fa72e81e4ad 100644
> > --- a/list-objects-filter-options.c
> > +++ b/list-objects-filter-options.c
> > @@ -252,16 +252,14 @@ void parse_list_objects_filter(
> > const char *arg)
> > {
> > struct strbuf errbuf = STRBUF_INIT;
> > - int parse_error;
> >
> > if (!filter_options->filter_spec.buf)
> > BUG("filter_options not properly initialized");
> >
> > if (!filter_options->choice) {
> > + if (gently_parse_list_objects_filter(filter_options, arg, &errbuf))
> > + die("%s", errbuf.buf);
> > strbuf_addstr(&filter_options->filter_spec, arg);
> > -
> > - parse_error = gently_parse_list_objects_filter(
> > - filter_options, arg, &errbuf);
> > } else {
> > struct list_objects_filter_options *sub;
> >
> > @@ -271,18 +269,17 @@ void parse_list_objects_filter(
> > */
> > transform_to_combine_type(filter_options);
> >
> > - strbuf_addch(&filter_options->filter_spec, '+');
> > - filter_spec_append_urlencode(filter_options, arg);
> > ALLOC_GROW_BY(filter_options->sub, filter_options->sub_nr, 1,
> > filter_options->sub_alloc);
> > sub = &filter_options->sub[filter_options->sub_nr - 1];
> >
> > list_objects_filter_init(sub);
> > - parse_error = gently_parse_list_objects_filter(sub, arg,
> > - &errbuf);
> > + if (gently_parse_list_objects_filter(sub, arg, &errbuf))
> > + die("%s", errbuf.buf);
>
> Do we actually have a test hitting this code path? I wanted to figure
> out why `filter_options->sub` wasn't leaky (I assume that's what you're
> talking about in your commit message), but I wasn't able to reproduce a
> scenario where we should die here.
You only refer to the second hunk, right? I couldn't find any code paths
hitting this, so this is more of a "Let's massage this code in the same
way" change. I don't want the next person to go through the same rabbit
hole as I had to go through :)
Patrick
