On 3/7/2022 11:22 AM, Ævar Arnfjörð Bjarmason wrote:

On Mon, Mar 07 2022, Derrick Stolee wrote:

quoted

On 3/7/2022 10:38 AM, Ævar Arnfjörð Bjarmason wrote:

quoted

On Wed, Feb 23 2022, Derrick Stolee via GitGitGadget wrote:

quoted

From: Derrick Stolee <redacted>

...

quoted

quoted

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.

quoted

quoted

 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...

I think your recommended "using_filter" is messy. Having this
struct be a pointer instead of embedded self-documents that it
is optional (and can be NULL) but that if it is non-NULL, then
it should be considered and valid.

Here, I'm focusing on not allowing a non-sensical state, such
as using_filter = 0 but filter is actually populated with a
valid filter. The possibility of this state means there is a
higher chance of introducing a bug over time by not keeping
these values coupled.

Thanks,
-Stolee