On Mon, Mar 07 2022, Derrick Stolee wrote: > 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@xxxxxxxxxx> > ... >>> 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...