On Wed, Mar 02, 2022 at 03:22:18PM -0500, Derrick Stolee wrote: > > + ret = load_pack_mtimes_file(mtimes_name, > > + p->num_objects, > > + &p->mtimes_map, > > + &p->mtimes_size); > > + if (ret) > > + goto cleanup; > > This looked odd to me, so I supposed that you had some code > that would be inserted between this 'goto cleanup' and the > 'cleanup:' label, but I did not find such an insertion in > the remaining patchs. This 'if' can be deleted. Thanks for spotting. My gut was that there must be something in the range-diff between this and the previous round, but there isn't. So this code has always been there. It likely comes from load_pack_revindex_from_disk(), which assigns the `revindex_data` member of `struct packed_git` after calling load_revindex_from_disk(), but only if it returned zero. We don't have to assign mtimes_data here (since it doesn't exist, and) because all of our reads into mtimes_map are offset by 3 to adjust for the width of the header. Anyway, we don't need this if statement here, so I'll drop it. Thanks, Taylor