Re: [PATCH 1/3] pack-objects: fix handling of multiple --filter options

From: Taylor Blau <me@ttaylorr.com>
Date: Nov 13, 2022, 05:01 UTC
Message-ID: <Y3B6PxNaZvmm5OD2@nand.local>
In-Reply-To: <Y2/QxOnM/7UI4R8c@coredump.intra.peff.net>

On Sat, Nov 12, 2022 at 11:58:44AM -0500, Jeff King wrote:

Show 16 quoted lines

> On Sat, Nov 12, 2022 at 11:44:00AM +0100, René Scharfe wrote:
>
> > Since 5cb28270a1 (pack-objects: lazily set up "struct rev_info", don't
> > leak, 2022-03-28) --filter options given to git pack-objects overrule
> > earlier ones, letting only the leftmost win and leaking the memory
> > allocated for earlier ones.  Fix that by only initializing the rev_info
> > struct once.
>
> Yikes. I wondered how we managed to miss this in the tests. Surely we
> have some examples that use multiple filters?
>
> I think the answer is that we do in t5616, but in practice they didn't
> hit this bug because of the way filter-specs are treated server-sid3.
> Even if we say "clone --filter=foo --filter=bar" on the client, the
> server upload-pack coalesces that into a single "combine:foo+bar"
> string, and that's what its pack-objects sees.

...Or in other words, clients aren't broken here because we always send "combine" filters when multiple `--filter` arguments are passed to `git clone`, `git fetch`, etc.

Is that always the case? I *think* so, but it would be nice to know if there was a way that clients would not send the `combine` filter with >1 filter.

Thanks, Taylor