Re: [PATCH 04/13] upload-pack: use 'struct upload_pack_data' in upload_pack()

From: Jeff King <peff@peff.net>
Date: May 15, 2020, 18:13 UTC
Message-ID: <20200515181319.GD3692649@coredump.intra.peff.net>
In-Reply-To: <fac9a9f1-7ee6-5138-9a9e-94e742b69f2c@gmail.com>

On Fri, May 15, 2020 at 06:30:23AM -0400, Derrick Stolee wrote:

> The control flow below was hard to parse from the diff because
> a whitespace line split up the "-" lines and the "+" lines.
> I reorder them here:

Using "-w" improves it a bit, because the packet_reader_init() becomes an anchor. But sadly the rest is hard to follow because of adding "data." to each variable reference.

I hoped --patience or --histogram might do better, but they don't. I think breaking it into a block like you did is the simplest way to see it.

> The major change is that the "options->advertise_refs" case
> now clears the data when before it did not. This seems like
> a good change to make.

Yeah, I think so, too. It looks like we were simply leaking the filter options before. I think we still leak the symref list, but it looks like that will get moved into upload_pack_data in a later patch, too.

So still looking good so far.

-Peff