git/list[1] front-page[2] threads[3] people[4] search[5] about
 

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

From
Christian Couder <christian.couder@gmail.com>
Date
Jun 4, 2020, 17:54 UTC
Message-ID
<20200604175450.17385-1-chriscool@tuxfamily.org>
In-Reply-To
<20200602041657.7132-1-chriscool@tuxfamily.org>

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 for his review of the first and second versions of this part 2:

V1: https://lore.kernel.org/git/20200527164742.23067-1-chriscool@tuxfamily.org/
V2: https://lore.kernel.org/git/20200602041657.7132-1-chriscool@tuxfamily.org/
Thanks to Jonathan Tan for his review of V2 too.
Compared to V2 the changes are the following:
  - patch 01/13 integrates Jonathan's commit message improvement
  - patch 07/13 integrates Jonathan's fixup patch which makes
    multi_ack enum values all-caps and fixes a spacing issue (2 spaces
    between "enum" and "{")

There is a lot of noise in the range-diff with V2, because V3 contains Peff's 'Acked-by:' and Junio's 'Signed-off-by', but here it is:

 1:  e5c31f30ec !  1:  12f8a9c953 upload-pack: actually use some upload_pack_data bitfields
    @@ Commit message
     
         As we cleanup 'upload-pack.c' by using 'struct upload_pack_data'
         more thoroughly, let's actually start using some bitfields of
    -    that struct, which were previously unused.
    +    that struct. These bitfields were introduced in 3145ea957d
    +    ("upload-pack: introduce fetch server command", 2018-03-15), but
    +    were never used.
     
         We could instead have just removed the following bitfields
         from the struct:
    @@ Commit message
         fixing a bug, but one nobody is likely to see, because it would
         imply the client sending different capabilities for each request.
     
    +    Helped-by: Jonathan Tan <jonathantanmy@google.com>
         Signed-off-by: Jeff King <peff@peff.net>
         Signed-off-by: Christian Couder <chriscool@tuxfamily.org>
    +    Acked-by: Jeff King <peff@peff.net>
    +    Signed-off-by: Junio C Hamano <gitster@pobox.com>
     
      ## upload-pack.c ##
     @@ upload-pack.c: static timestamp_t oldest_have;
 2:  225be644f4 !  2:  b74c2f4d64 upload-pack: annotate upload_pack_data fields
    @@ Commit message
         ones only for protocol v2.
     
         Signed-off-by: Christian Couder <chriscool@tuxfamily.org>
    +    Acked-by: Jeff King <peff@peff.net>
    +    Signed-off-by: Junio C Hamano <gitster@pobox.com>
     
      ## upload-pack.c ##
     @@ upload-pack.c: static int allow_ref_in_want;
 3:  5adca8d70c !  3:  05cb66320a upload-pack: move static vars to upload_pack_data
    @@ Commit message
         now is more coherent and cleaner.
     
         Signed-off-by: Christian Couder <chriscool@tuxfamily.org>
    +    Acked-by: Jeff King <peff@peff.net>
    +    Signed-off-by: Junio C Hamano <gitster@pobox.com>
     
      ## upload-pack.c ##
     @@
 4:  1507a8134a !  4:  be32b64b0a upload-pack: move use_sideband to upload_pack_data
    @@ Commit message
         definition.
     
         Signed-off-by: Christian Couder <chriscool@tuxfamily.org>
    +    Acked-by: Jeff King <peff@peff.net>
    +    Signed-off-by: Junio C Hamano <gitster@pobox.com>
     
      ## upload-pack.c ##
     @@ upload-pack.c: static unsigned int allow_unadvertised_object_request;
 5:  0ef8d28958 !  5:  154651241d upload-pack: move filter_capability_requested to upload_pack_data
    @@ Commit message
         now is more coherent and cleaner.
     
         Signed-off-by: Christian Couder <chriscool@tuxfamily.org>
    +    Acked-by: Jeff King <peff@peff.net>
    +    Signed-off-by: Junio C Hamano <gitster@pobox.com>
     
      ## upload-pack.c ##
     @@ upload-pack.c: static struct object_array extra_edge_obj;
 6:  319c09ce02 !  6:  ba6999dc74 upload-pack: move multi_ack to upload_pack_data
    @@ Commit message
         now is more coherent and cleaner.
     
         Signed-off-by: Christian Couder <chriscool@tuxfamily.org>
    +    Acked-by: Jeff King <peff@peff.net>
    +    Signed-off-by: Junio C Hamano <gitster@pobox.com>
     
      ## upload-pack.c ##
     @@
 7:  cd3a0a1e5c !  7:  988e2e1d9a upload-pack: change multi_ack to an enum
    @@ Commit message
     
         This will make it clear which values this variable can take.
     
    +    Helped-by: Jonathan Tan <jonathantanmy@google.com>
         Signed-off-by: Christian Couder <chriscool@tuxfamily.org>
    +    Acked-by: Jeff King <peff@peff.net>
    +    Signed-off-by: Junio C Hamano <gitster@pobox.com>
     
      ## upload-pack.c ##
     @@ upload-pack.c: struct upload_pack_data {
    @@ upload-pack.c: struct upload_pack_data {
      
        unsigned int timeout;                                   /* v0 only */
     -  int multi_ack;                                          /* v0 only */
    -+  enum  {
    -+          no_multi_ack = 0,
    -+          multi_ack = 1,
    -+          multi_ack_detailed = 2
    ++  enum {
    ++          NO_MULTI_ACK = 0,
    ++          MULTI_ACK = 1,
    ++          MULTI_ACK_DETAILED = 2
     +  } multi_ack;                                            /* v0 only */
      
        /* 0 for no sideband, otherwise DEFAULT_PACKET_MAX or LARGE_PACKET_MAX */
    @@ upload-pack.c: static int get_common_commits(struct upload_pack_data *data,
      
                if (packet_reader_read(reader) != PACKET_READ_NORMAL) {
     -                  if (data->multi_ack == 2
    -+                  if (data->multi_ack == multi_ack_detailed
    ++                  if (data->multi_ack == MULTI_ACK_DETAILED
                            && got_common
                            && !got_other
                            && ok_to_give_up(&data->have_obj, &data->want_obj)) {
    @@ upload-pack.c: static int get_common_commits(struct upload_pack_data *data,
                                    && ok_to_give_up(&data->have_obj, &data->want_obj)) {
                                        const char *hex = oid_to_hex(&oid);
     -                                  if (data->multi_ack == 2) {
    -+                                  if (data->multi_ack == multi_ack_detailed) {
    ++                                  if (data->multi_ack == MULTI_ACK_DETAILED) {
                                                sent_ready = 1;
                                                packet_write_fmt(1, "ACK %s ready\n", hex);
                                        } else
    @@ upload-pack.c: static int get_common_commits(struct upload_pack_data *data,
                                got_common = 1;
                                oid_to_hex_r(last_hex, &oid);
     -                          if (data->multi_ack == 2)
    -+                          if (data->multi_ack == multi_ack_detailed)
    ++                          if (data->multi_ack == MULTI_ACK_DETAILED)
                                        packet_write_fmt(1, "ACK %s common\n", last_hex);
                                else if (data->multi_ack)
                                        packet_write_fmt(1, "ACK %s continue\n", last_hex);
    @@ upload-pack.c: static void receive_needs(struct upload_pack_data *data,
                        data->deepen_relative = 1;
                if (parse_feature_request(features, "multi_ack_detailed"))
     -                  data->multi_ack = 2;
    -+                  data->multi_ack = multi_ack_detailed;
    ++                  data->multi_ack = MULTI_ACK_DETAILED;
                else if (parse_feature_request(features, "multi_ack"))
     -                  data->multi_ack = 1;
    -+                  data->multi_ack = multi_ack;
    ++                  data->multi_ack = MULTI_ACK;
                if (parse_feature_request(features, "no-done"))
                        data->no_done = 1;
                if (parse_feature_request(features, "thin-pack"))
 8:  c23e37a8da !  8:  8dcd7dddc2 upload-pack: pass upload_pack_data to upload_pack_config()
    @@ Commit message
         that are set in upload_pack_config() into 'upload_pack_data'.
     
         Signed-off-by: Christian Couder <chriscool@tuxfamily.org>
    +    Acked-by: Jeff King <peff@peff.net>
    +    Signed-off-by: Junio C Hamano <gitster@pobox.com>
     
      ## upload-pack.c ##
     @@ upload-pack.c: static int find_symref(const char *refname, const struct object_id *oid,
 9:  26893a4098 !  9:  8c165eef8f upload-pack: move keepalive to upload_pack_data
    @@ Commit message
         It is used by code common to protocol v0 and protocol v2.
     
         Signed-off-by: Christian Couder <chriscool@tuxfamily.org>
    +    Acked-by: Jeff King <peff@peff.net>
    +    Signed-off-by: Junio C Hamano <gitster@pobox.com>
     
      ## upload-pack.c ##
     @@ upload-pack.c: static timestamp_t oldest_have;
    @@ upload-pack.c: struct upload_pack_data {
     +  int keepalive;
      
        unsigned int timeout;                                   /* v0 only */
    -   enum  {
    +   enum {
     @@ upload-pack.c: static void upload_pack_data_init(struct upload_pack_data *data)
        data->shallows = shallows;
        data->deepen_not = deepen_not;
10:  2fa59d6c89 ! 10:  17ac2a015a upload-pack: move allow_filter to upload_pack_data
    @@ Commit message
         It is used by both protocol v0 and protocol v2 code.
     
         Signed-off-by: Christian Couder <chriscool@tuxfamily.org>
    +    Acked-by: Jeff King <peff@peff.net>
    +    Signed-off-by: Junio C Hamano <gitster@pobox.com>
     
      ## upload-pack.c ##
     @@ upload-pack.c: static int shallow_nr;
11:  d501b0f694 ! 11:  74245639d8 upload-pack: move allow_ref_in_want to upload_pack_data
    @@ Commit message
         It is used only by protocol v2 code.
     
         Signed-off-by: Christian Couder <chriscool@tuxfamily.org>
    +    Acked-by: Jeff King <peff@peff.net>
    +    Signed-off-by: Junio C Hamano <gitster@pobox.com>
     
      ## upload-pack.c ##
     @@ upload-pack.c: static int shallow_nr;
12:  90513f0543 ! 12:  d866ea2741 upload-pack: move allow_sideband_all to upload_pack_data
    @@ Commit message
         It is used only by protocol v2 code.
     
         Signed-off-by: Christian Couder <chriscool@tuxfamily.org>
    +    Acked-by: Jeff King <peff@peff.net>
    +    Signed-off-by: Junio C Hamano <gitster@pobox.com>
     
      ## upload-pack.c ##
     @@ upload-pack.c: static int shallow_nr;
13:  f2851482f5 ! 13:  fce53edae4 upload-pack: move pack_objects_hook to upload_pack_data
    @@ Commit message
         While at it let's also free() it in upload_pack_data_clear().
     
         Signed-off-by: Christian Couder <chriscool@tuxfamily.org>
    +    Acked-by: Jeff King <peff@peff.net>
    +    Signed-off-by: Junio C Hamano <gitster@pobox.com>
     
      ## upload-pack.c ##
     @@ upload-pack.c: static timestamp_t oldest_have;
Christian Couder (12):
  upload-pack: annotate upload_pack_data fields
  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 | 199 ++++++++++++++++++++++++++++----------------------
 1 file changed, 110 insertions(+), 89 deletions(-)
-- 
2.27.0.rc0.26.gf2851482f5
Previous: Christian CouderNext: Christian Couder
Message 77 of 108 in “upload-pack: use 'struct upload_pack_data' thoroughly, part 1”
  1. 00/13 upload-pack: use 'struct upload_pack_data' thoroughly, part 1Christian Couder, May 15, 2020
  2. 01/13 upload-pack: remove unused 'wants' from upload_pack_dataChristian Couder, May 15, 2020
  3. Jeff KingMay 15, 2020
  4. 03/13 upload-pack: move 'struct upload_pack_data' aroundChristian Couder, May 15, 2020
  5. Jeff KingMay 15, 2020
  6. 02/13 upload-pack: move {want,have}_obj to upload_pack_dataChristian Couder, May 15, 2020
  7. Jeff KingMay 15, 2020
  8. 06/13 upload-pack: pass upload_pack_data to receive_needs()Christian Couder, May 15, 2020
  9. Jeff KingMay 15, 2020
  10. 04/13 upload-pack: use 'struct upload_pack_data' in upload_pack()Christian Couder, May 15, 2020
  11. Derrick StoleeMay 15, 2020
  12. Jeff KingMay 15, 2020
  13. 05/13 upload-pack: pass upload_pack_data to get_common_commits()Christian Couder, May 15, 2020
  14. Jeff KingMay 15, 2020
  15. Jeff KingMay 15, 2020
  16. 08/13 upload-pack: move symref to upload_pack_dataChristian Couder, May 15, 2020
  17. Jeff KingMay 15, 2020
  18. 07/13 upload-pack: use upload_pack_data writer in receive_needs()Christian Couder, May 15, 2020
  19. Jeff KingMay 15, 2020
  20. 09/13 upload-pack: pass upload_pack_data to send_ref()Christian Couder, May 15, 2020
  21. Jeff KingMay 15, 2020
  22. 12/13 upload-pack: pass upload_pack_data to create_pack_file()Christian Couder, May 15, 2020
  23. 13/13 upload-pack: use upload_pack_data fields in receive_needs()Christian Couder, May 15, 2020
  24. Jeff KingMay 15, 2020
  25. 11/13 upload-pack: remove static variable 'stateless_rpc'Christian Couder, May 15, 2020
  26. Jeff KingMay 15, 2020
  27. 10/13 upload-pack: pass upload_pack_data to check_non_tip()Christian Couder, May 15, 2020
  28. Derrick StoleeMay 15, 2020
  29. Christian CouderMay 19, 2020
  30. Jeff KingMay 15, 2020
  31. Jeff KingMay 15, 2020
  32. Christian CouderMay 19, 2020
  33. 00/12 upload-pack: use 'struct upload_pack_data' thoroughly, part 2Christian Couder, May 27, 2020
  34. 02/12 upload-pack: move static vars to upload_pack_dataChristian Couder, May 27, 2020
  35. Jeff KingMay 27, 2020
  36. Christian CouderJun 2, 2020
  37. 01/12 upload-pack: actually use some upload_pack_data bitfieldsChristian Couder, May 27, 2020
  38. 03/12 upload-pack: move use_sideband to upload_pack_dataChristian Couder, May 27, 2020
  39. Jeff KingMay 27, 2020
  40. 04/12 upload-pack: move filter_capability_requested to upload_pack_dataChristian Couder, May 27, 2020
  41. Jeff KingMay 27, 2020
  42. 05/12 upload-pack: move multi_ack to upload_pack_dataChristian Couder, May 27, 2020
  43. 06/12 upload-pack: change multi_ack to an enumChristian Couder, May 27, 2020
  44. Jeff KingMay 27, 2020
  45. 09/12 upload-pack: move allow_filter to upload_pack_dataChristian Couder, May 27, 2020
  46. Jeff KingMay 27, 2020
  47. 08/12 upload-pack: move keepalive to upload_pack_dataChristian Couder, May 27, 2020
  48. Jeff KingMay 27, 2020
  49. 12/12 upload-pack: move pack_objects_hook to upload_pack_dataChristian Couder, May 27, 2020
  50. Jeff KingMay 27, 2020
  51. 11/12 upload-pack: move allow_sideband_all to upload_pack_dataChristian Couder, May 27, 2020
  52. 10/12 upload-pack: move allow_ref_in_want to upload_pack_dataChristian Couder, May 27, 2020
  53. Jeff KingMay 27, 2020
  54. 07/12 upload-pack: pass upload_pack_data to upload_pack_config()Christian Couder, May 27, 2020
  55. Jeff KingMay 27, 2020
  56. Jeff KingMay 27, 2020
  57. Junio C HamanoMay 27, 2020
  58. 00/13 upload-pack: use 'struct upload_pack_data' thoroughly, part 2Christian Couder, Jun 2, 2020
  59. 01/13 upload-pack: actually use some upload_pack_data bitfieldsChristian Couder, Jun 2, 2020
  60. 02/13 upload-pack: annotate upload_pack_data fieldsChristian Couder, Jun 2, 2020
  61. 04/13 upload-pack: move use_sideband to upload_pack_dataChristian Couder, Jun 2, 2020
  62. 03/13 upload-pack: move static vars to upload_pack_dataChristian Couder, Jun 2, 2020
  63. Jeff KingJun 2, 2020
  64. 05/13 upload-pack: move filter_capability_requested to upload_pack_dataChristian Couder, Jun 2, 2020
  65. 06/13 upload-pack: move multi_ack to upload_pack_dataChristian Couder, Jun 2, 2020
  66. 08/13 upload-pack: pass upload_pack_data to upload_pack_config()Christian Couder, Jun 2, 2020
  67. 07/13 upload-pack: change multi_ack to an enumChristian Couder, Jun 2, 2020
  68. 11/13 upload-pack: move allow_ref_in_want to upload_pack_dataChristian Couder, Jun 2, 2020
  69. 10/13 upload-pack: move allow_filter to upload_pack_dataChristian Couder, Jun 2, 2020
  70. 09/13 upload-pack: move keepalive to upload_pack_dataChristian Couder, Jun 2, 2020
  71. 12/13 upload-pack: move allow_sideband_all to upload_pack_dataChristian Couder, Jun 2, 2020
  72. 13/13 upload-pack: move pack_objects_hook to upload_pack_dataChristian Couder, Jun 2, 2020
  73. Jeff KingJun 2, 2020
  74. Junio C HamanoJun 2, 2020
  75. fixup! upload-pack: change multi_ack to an enumJonathan Tan, Jun 2, 2020
  76. Christian CouderJun 2, 2020
  77. 00/13 upload-pack: use 'struct upload_pack_data' thoroughly, part 2Christian Couder, Jun 4, 2020
  78. 01/13 upload-pack: actually use some upload_pack_data bitfieldsChristian Couder, Jun 4, 2020
  79. 02/13 upload-pack: annotate upload_pack_data fieldsChristian Couder, Jun 4, 2020
  80. 03/13 upload-pack: move static vars to upload_pack_dataChristian Couder, Jun 4, 2020
  81. 04/13 upload-pack: move use_sideband to upload_pack_dataChristian Couder, Jun 4, 2020
  82. 08/13 upload-pack: pass upload_pack_data to upload_pack_config()Christian Couder, Jun 4, 2020
  83. 07/13 upload-pack: change multi_ack to an enumChristian Couder, Jun 4, 2020
  84. 09/13 upload-pack: move keepalive to upload_pack_dataChristian Couder, Jun 4, 2020
  85. 10/13 upload-pack: move allow_filter to upload_pack_dataChristian Couder, Jun 4, 2020
  86. 11/13 upload-pack: move allow_ref_in_want to upload_pack_dataChristian Couder, Jun 4, 2020
  87. 12/13 upload-pack: move allow_sideband_all to upload_pack_dataChristian Couder, Jun 4, 2020
  88. 13/13 upload-pack: move pack_objects_hook to upload_pack_dataChristian Couder, Jun 4, 2020
  89. 05/13 upload-pack: move filter_capability_requested to upload_pack_dataChristian Couder, Jun 4, 2020
  90. 06/13 upload-pack: move multi_ack to upload_pack_dataChristian Couder, Jun 4, 2020
  91. Junio C HamanoJun 4, 2020
  92. Christian CouderJun 5, 2020
  93. 00/14 upload-pack: use 'struct upload_pack_data' thoroughly, part 3Christian Couder, Jun 11, 2020
  94. 01/14 upload-pack: pass upload_pack_data to send_shallow_list()Christian Couder, Jun 11, 2020
  95. 03/14 upload-pack: pass upload_pack_data to deepen_by_rev_list()Christian Couder, Jun 11, 2020
  96. 04/14 upload-pack: pass upload_pack_data to send_unshallow()Christian Couder, Jun 11, 2020
  97. 02/14 upload-pack: pass upload_pack_data to deepen()Christian Couder, Jun 11, 2020
  98. 05/14 upload-pack: move shallow_nr to upload_pack_dataChristian Couder, Jun 11, 2020
  99. 07/14 upload-pack: move allow_unadvertised_object_request to upload_pack_dataChristian Couder, Jun 11, 2020
  100. 08/14 upload-pack: change allow_unadvertised_object_request to an enumChristian Couder, Jun 11, 2020
  101. 06/14 upload-pack: move extra_edge_obj to upload_pack_dataChristian Couder, Jun 11, 2020
  102. 10/14 upload-pack: pass upload_pack_data to send_acks()Christian Couder, Jun 11, 2020
  103. 11/14 upload-pack: pass upload_pack_data to ok_to_give_up()Christian Couder, Jun 11, 2020
  104. 12/14 upload-pack: pass upload_pack_data to got_oid()Christian Couder, Jun 11, 2020
  105. 09/14 upload-pack: pass upload_pack_data to process_haves()Christian Couder, Jun 11, 2020
  106. 14/14 upload-pack: refactor common code into do_got_oid()Christian Couder, Jun 11, 2020
  107. 13/14 upload-pack: move oldest_have to upload_pack_dataChristian Couder, Jun 11, 2020
  108. Jonathan TanJun 11, 2020

Read the whole thread, see it on lore, or plain text.

$ cat FOOTERMessages come from the public archive at lore.kernel.org/git, fetched every hour. The front page is chosen and written each morning by an AI editor and can be wrong; the threads themselves are the record. About and API. For agents: an MCP server at https://gitlist.dev/mcp, and any thread, story or person page as Markdown by adding .md to its URL (or sending Accept: text/markdown). Details in /llms.txt.