> 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 [1], linked from that commit's email [2]. (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. [1] https://public-inbox.org/git/20181218010811.143608-1-jonathantanmy@xxxxxxxxxx/ [2] https://public-inbox.org/git/20190110193645.34080-1-jonathantanmy@xxxxxxxxxx/ > The problem with dangling alternate_shallow_file is also from that > commit. 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.
