Re: [PATCH 08/11] bundle: parse filter capability
- From
Ævar Arnfjörð Bjarmason <avarab@gmail.com>
- Date
- Mar 7, 2022, 16:22 UTC
- Message-ID
- <220307.86pmmxybmz.gmgdl@evledraar.gmail.com>
- In-Reply-To
- <142219bd-5047-a27d-d53f-2ec2f6cb9c08@github.com>
On Mon, Mar 07 2022, Derrick Stolee wrote:
Show 44 quoted lines
> On 3/7/2022 10:38 AM, Ævar Arnfjörð Bjarmason wrote:
>>
>> On Wed, Feb 23 2022, Derrick Stolee via GitGitGadget wrote:
>>
>>> From: Derrick Stolee <derrickstolee@github.com>
> ...
>>> diff --git a/bundle.h b/bundle.h
>>> index 06009fe6b1f..eb026153d56 100644
>>> --- a/bundle.h
>>> +++ b/bundle.h
>>> @@ -5,11 +5,14 @@
>>> #include "cache.h"
>>> #include "string-list.h"
>>>
>>> +struct list_objects_filter_options;
>>> +
>>
>> For the other ones we include the relevant header, do the same here (or
>> if there's a need to not do it, do we need it for the rest too?)
>
> The others are .c files that require looking into the struct. This
> declaration is all that's required for this header file.
>
>>> struct bundle_header {
>>> unsigned version;
>>> struct string_list prerequisites;
>>> struct string_list references;
>>> const struct git_hash_algo *hash_algo;
>>> + struct list_objects_filter_options *filter;
>>> };
>>
>> I haven't tried, but any reason this needs to be a *filter
>> v.s. embedding it in the struct?
>>
>> Then we'd just need list_objects_filter_release() and not the free() as
>> well.
>>
>> Is it because you're piggy-backing on "if (header->filter)" as "do we
>> have it" state, better to check .nr?
>
> Yes. I replied to Junio before that there is some assumption in
> the filtering code that the .nr == 0 case is listed as a BUG()
> so we would possibly be breaking expectations in a different
> way doing the embedded version.Having an "unsigned int using_filter:1" or whatever IMO makes that much clearer than needing to carefully eyeball code that's already using embedded structs & see why the one exception that's malloc'd is because of that or some other reason...