On Fri, May 15, 2020 at 12:04:49PM +0200, Christian Couder wrote: > As we cleanup 'upload-pack.c' by using 'struct upload_pack_data' > more thoroughly, we are passing around that struct to many > functions, so let's also pass 'struct string_list symref' around > at the same time by moving it from a local variable in > upload_pack() into a field of 'struct upload_pack_data'. Hmm, do we really benefit here at all? The symref list is a local implementation detail of upload_pack(), and v2 would not use it at all. Nor does it ever survive past the function in which it's declared. Perhaps it would help if we were able to send upload_pack_data along to for_each_ref(), as you do in the next patch, and that let us make use of upload_pack_data to access other variables. But it doesn't look like we ever do. In light of that, is it worth it? I think we _could_ switch send_ref() (the callback for for_each_ref()) to use the packet-writer struct. Perhaps that would make it worth doing. -Peff