On Tue, Mar 26, 2019 at 10:37:06AM -0700, Jonathan Tan wrote: > > Perhaps it's worth passing down the shallows array we get from the > > caller of fetch_pack(). Something like the patch below (I think it is > > never NULL, which means in your patch 1 you can simplify the conditional > > for the BUG). > > [snip patch] > > You're right that it is never NULL - I have removed that check. As for > passing down the shallows array that we get from the caller of > fetch_pack(), that would get confusing because we end up modifying the > shallows array in some code paths, and the transport is sometimes reused > (for example, when backfilling tags). I have instead made a > shallows_scratch variable in fetch_pack(), and made it pass it down > (like in the diff you provided). Yeah, I confess to having spent quite a few minutes trying to figure out the difference between "shallows" and "shallow_info", whether one wrote into the other, and who was responsible for filling each in. So I will not complain if you have a way of writing it that is less confusing. :) -Peff
