mbox series

[00/12] upload-pack: use 'struct upload_pack_data' thoroughly, part 2

Message ID 20200527164742.23067-1-chriscool@tuxfamily.org (mailing list archive)
Headers show
Series upload-pack: use 'struct upload_pack_data' thoroughly, part 2 | expand

Message

Christian Couder May 27, 2020, 4:47 p.m. UTC 
This patch series is the second part of an effort to move all static
variables in 'upload-pack.c' into 'struct upload_pack_data'.

It is based on 'cc/upload-pack-data' which contains "part 1" of this
effort. (See also: https://lore.kernel.org/git/20200515100454.14486-1-chriscool@tuxfamily.org/)

A part 3 will follow with the rest of the patches needed to get rid of
the static variables left after this patch series.

Thanks to Peff and Stolee for their review and help.

Christian Couder (11):
  upload-pack: move static vars to upload_pack_data
  upload-pack: move use_sideband to upload_pack_data
  upload-pack: move filter_capability_requested to upload_pack_data
  upload-pack: move multi_ack to upload_pack_data
  upload-pack: change multi_ack to an enum
  upload-pack: pass upload_pack_data to upload_pack_config()
  upload-pack: move keepalive to upload_pack_data
  upload-pack: move allow_filter to upload_pack_data
  upload-pack: move allow_ref_in_want to upload_pack_data
  upload-pack: move allow_sideband_all to upload_pack_data
  upload-pack: move pack_objects_hook to upload_pack_data

Jeff King (1):
  upload-pack: actually use some upload_pack_data bitfields

 upload-pack.c | 185 +++++++++++++++++++++++++++-----------------------
 1 file changed, 101 insertions(+), 84 deletions(-)

Comments

Jeff King May 27, 2020, 6:57 p.m. UTC | #1 
On Wed, May 27, 2020 at 06:47:30PM +0200, Christian Couder wrote:

> This patch series is the second part of an effort to move all static
> variables in 'upload-pack.c' into 'struct upload_pack_data'.
> 
> It is based on 'cc/upload-pack-data' which contains "part 1" of this
> effort. (See also: https://lore.kernel.org/git/20200515100454.14486-1-chriscool@tuxfamily.org/)
> 
> A part 3 will follow with the rest of the patches needed to get rid of
> the static variables left after this patch series.

Thanks again for working on this. It mostly looks good, but I left a few
comments, mostly about commit messages or comments. :)

One thing I think we _could_ do is to switch v2 to only reading the
config once, instead of once per request. And then all of those config
values could remain where they are, as they wouldn't need to be cleared
or reset. But I doubt the cost of parsing config per-request is
noticeable in practice, so I'm happy with it either way.

-Peff
Junio C Hamano May 27, 2020, 8:41 p.m. UTC | #2 
Jeff King <peff@peff.net> writes:

> One thing I think we _could_ do is to switch v2 to only reading the
> config once, instead of once per request. And then all of those config
> values could remain where they are, as they wouldn't need to be cleared
> or reset. But I doubt the cost of parsing config per-request is
> noticeable in practice, so I'm happy with it either way.

Yeah, I agree that that would not be an optimization for
performance, but the value of doing it primarily lies in
gaining the conceptual clarity in the resulting code.

Thanks, both.