Re: [PATCH] fetch-pack: clear alternate shallow when complete
- Date: Tue, 5 Feb 2019 08:26:36 -0800
- From: Jonathan Tan <jonathantanmy@xxxxxxxxxx>
- Subject: Re: [PATCH] fetch-pack: clear alternate shallow when complete
> On Mon, Feb 4, 2019 at 7:06 AM brian m. carlson
> <sandals@xxxxxxxxxxxxxxxxxxxx> wrote:
> > When we write an alternate shallow file in update_shallow, we write it
> > into the lock file. The string stored in alternate_shallow_file is
> > copied from the lock file path, but it is freed the moment that the lock
> > file is closed, since we call strbuf_release to free that path.
> > This used to work, since we did not invoke git index-pack more than
> > once, but now that we do, we reuse the freed memory. Ensure we reset the
> > value to NULL to avoid using freed memory. git index-pack will read the
> > repository's shallow file, which will have been updated with the correct
> > information.
> It may be worth mentioning bd0b42aed3 (fetch-pack: do not take shallow
> lock unnecessarily - 2019-01-10). I believe this is the same problem
> and a full solution was suggested but not implemented in that commit.
For reference, the full solution is , linked from that commit's email
. (Looking back, I probably should have included all the information
below the "---" in the commit message proper.) The full solution is more
related to shallow locks, though, not alternate_shallow_file.
> The problem with dangling alternate_shallow_file is also from that
You're right - thanks for noticing this.
> When line_received is false at the end of
> receive_shallow_info(), we should clear alternate_shallow_file. I'm
> still debating myself whether we should clear alternate_shallow_file
> in receive_shallow_info() in addition to your changes (which is good
> hygiene anyway) to keep the setup steps of do_fetch_pack() and
> do_fetch_pack_v2() aligned.
Clearing alternate_shallow_file when line_received is false at the end
of receive_shallow_info() sounds like a good idea to me.