On Fri, May 15, 2020 at 12:04:54PM +0200, Christian Couder wrote: > As we cleanup 'upload-pack.c' by using 'struct upload_pack_data' > more thoroughly, let's use fields from this struct in > receive_needs(), instead of local variables with the same name > and purpose. OK, makes sense. These are purely local in v0, but it's nice for us to match v2 better (rather than somebody looking at v0 wondering why data.shallows is never used). > - if (send_shallow_list(&data->writer, depth, deepen_rev_list, deepen_since, > - &deepen_not, deepen_relative, &shallows, > + if (send_shallow_list(&data->writer, > + data->depth, > + data->deepen_rev_list, > + data->deepen_since, > + &data->deepen_not, > + data->deepen_relative, > + &data->shallows, > &data->want_obj)) > packet_flush(1); > - object_array_clear(&shallows); > } We can drop this final cleanup step because it's now covered by upload_pack_data_clear(). I wondered if any callers would care that we don't clear it until later, but I guess it couldn't possibly matter: they would not have had access to this "shallows" variable in the first place, since it was local. So this is obviously correct. -Peff