From: Derrick Stolee <stolee@gmail.com>
To: Christian Couder <christian.couder@gmail.com>, git@vger.kernel.org
Cc: Junio C Hamano <gitster@pobox.com>,
Derrick Stolee <dstolee@microsoft.com>, Jeff King <peff@peff.net>,
Taylor Blau <me@ttaylorr.com>,
Jonathan Tan <jonathantanmy@google.com>,
Jonathan Nieder <jrnieder@gmail.com>,
Christian Couder <chriscool@tuxfamily.org>
Subject: Re: [PATCH 04/13] upload-pack: use 'struct upload_pack_data' in upload_pack()
Date: Fri, 15 May 2020 06:30:23 -0400 [thread overview]
Message-ID: <fac9a9f1-7ee6-5138-9a9e-94e742b69f2c@gmail.com> (raw)
In-Reply-To: <20200515100454.14486-5-chriscool@tuxfamily.org>
On 5/15/2020 6:04 AM, Christian Couder wrote:
> As we cleanup 'upload-pack.c' by using 'struct upload_pack_data'
> more thoroughly, let's use 'struct upload_pack_data' in
> upload_pack().
>
> This will make it possible in followup commits to remove a lot
> of static variables and local variables that have the same name
> and purpose as fields in 'struct upload_pack_data'. This will
> also make upload_pack() work in a more similar way as
> upload_pack_v2().
>
> Signed-off-by: Christian Couder <chriscool@tuxfamily.org>
> ---
> upload-pack.c | 32 +++++++++++++++++---------------
> 1 file changed, 17 insertions(+), 15 deletions(-)
>
> diff --git a/upload-pack.c b/upload-pack.c
> index 9aeb3477c9..cb336c5713 100644
> --- a/upload-pack.c
> +++ b/upload-pack.c
> @@ -1144,18 +1144,17 @@ static int upload_pack_config(const char *var, const char *value, void *unused)
> void upload_pack(struct upload_pack_options *options)
> {
> struct string_list symref = STRING_LIST_INIT_DUP;
> - struct object_array want_obj = OBJECT_ARRAY_INIT;
> struct packet_reader reader;
> - struct list_objects_filter_options filter_options;
> + struct upload_pack_data data;
>
> stateless_rpc = options->stateless_rpc;
> timeout = options->timeout;
> daemon_mode = options->daemon_mode;
>
> - memset(&filter_options, 0, sizeof(filter_options));
> -
I checked that upload_pack_data_init() runs memset(), which
initializes the memory for data.filter_options. Thanks.
> git_config(upload_pack_config, NULL);
>
> + upload_pack_data_init(&data);
> +
> head_ref_namespaced(find_symref, &symref);
>
> if (options->advertise_refs || !stateless_rpc) {
> @@ -1169,21 +1168,24 @@ void upload_pack(struct upload_pack_options *options)
The control flow below was hard to parse from the diff because
a whitespace line split up the "-" lines and the "+" lines.
I reorder them here:
Old code:
> - if (options->advertise_refs)
> - return;
>
> - packet_reader_init(&reader, 0, NULL, 0,
> - PACKET_READ_CHOMP_NEWLINE |
> - PACKET_READ_DIE_ON_ERR_PACKET);
>
> - receive_needs(&reader, &want_obj, &filter_options);
> - if (want_obj.nr) {
> - struct object_array have_obj = OBJECT_ARRAY_INIT;
> - get_common_commits(&reader, &have_obj, &want_obj);
> - create_pack_file(&have_obj, &want_obj, &filter_options);
> }
> - list_objects_filter_release(&filter_options);
> }
New code:
> + if (!options->advertise_refs) {
> + packet_reader_init(&reader, 0, NULL, 0,
> + PACKET_READ_CHOMP_NEWLINE |
> + PACKET_READ_DIE_ON_ERR_PACKET);
>
> + receive_needs(&reader, &data.want_obj, &data.filter_options);
> + if (data.want_obj.nr) {
> + get_common_commits(&reader,
> + &data.have_obj,
> + &data.want_obj);
> + create_pack_file(&data.have_obj,
> + &data.want_obj,
> + &data.filter_options);
> + }
> }
>
> + upload_pack_data_clear(&data);
> }
The major change is that the "options->advertise_refs" case
now clears the data when before it did not. This seems like
a good change to make.
Also, the code that has now been surrounded by an if block
isn't so large as to justify a "goto cleanup" case.
Thanks,
-Stolee
next prev parent reply other threads:[~2020-05-15 10:30 UTC|newest]
Thread overview: 108+ messages / expand[flat|nested] mbox.gz Atom feed top
2020-05-15 10:04 [PATCH 00/13] upload-pack: use 'struct upload_pack_data' thoroughly, part 1 Christian Couder
2020-05-15 10:04 ` [PATCH 01/13] upload-pack: remove unused 'wants' from upload_pack_data Christian Couder
2020-05-15 18:03 ` Jeff King
2020-05-15 10:04 ` [PATCH 02/13] upload-pack: move {want,have}_obj to upload_pack_data Christian Couder
2020-05-15 18:05 ` Jeff King
2020-05-15 10:04 ` [PATCH 03/13] upload-pack: move 'struct upload_pack_data' around Christian Couder
2020-05-15 18:05 ` Jeff King
2020-05-15 10:04 ` [PATCH 04/13] upload-pack: use 'struct upload_pack_data' in upload_pack() Christian Couder
2020-05-15 10:30 ` Derrick Stolee [this message]
2020-05-15 18:13 ` Jeff King
2020-05-15 10:04 ` [PATCH 05/13] upload-pack: pass upload_pack_data to get_common_commits() Christian Couder
2020-05-15 18:17 ` Jeff King
2020-05-15 18:36 ` Jeff King
2020-05-15 10:04 ` [PATCH 06/13] upload-pack: pass upload_pack_data to receive_needs() Christian Couder
2020-05-15 18:20 ` Jeff King
2020-05-15 10:04 ` [PATCH 07/13] upload-pack: use upload_pack_data writer in receive_needs() Christian Couder
2020-05-15 18:23 ` Jeff King
2020-05-15 10:04 ` [PATCH 08/13] upload-pack: move symref to upload_pack_data Christian Couder
2020-05-15 18:28 ` Jeff King
2020-05-15 10:04 ` [PATCH 09/13] upload-pack: pass upload_pack_data to send_ref() Christian Couder
2020-05-15 18:33 ` Jeff King
2020-05-15 10:04 ` [PATCH 10/13] upload-pack: pass upload_pack_data to check_non_tip() Christian Couder
2020-05-15 10:04 ` [PATCH 11/13] upload-pack: remove static variable 'stateless_rpc' Christian Couder
2020-05-15 18:38 ` Jeff King
2020-05-15 10:04 ` [PATCH 12/13] upload-pack: pass upload_pack_data to create_pack_file() Christian Couder
2020-05-15 10:04 ` [PATCH 13/13] upload-pack: use upload_pack_data fields in receive_needs() Christian Couder
2020-05-15 18:42 ` Jeff King
2020-05-15 10:43 ` [PATCH 00/13] upload-pack: use 'struct upload_pack_data' thoroughly, part 1 Derrick Stolee
2020-05-19 9:49 ` Christian Couder
2020-05-15 18:47 ` Jeff King
2020-05-15 18:55 ` Jeff King
2020-05-19 9:44 ` Christian Couder
2020-05-27 16:47 ` [PATCH 00/12] upload-pack: use 'struct upload_pack_data' thoroughly, part 2 Christian Couder
2020-05-27 16:47 ` [PATCH 01/12] upload-pack: actually use some upload_pack_data bitfields Christian Couder
2020-05-27 16:47 ` [PATCH 02/12] upload-pack: move static vars to upload_pack_data Christian Couder
2020-05-27 18:06 ` Jeff King
2020-06-02 4:19 ` Christian Couder
2020-05-27 16:47 ` [PATCH 03/12] upload-pack: move use_sideband " Christian Couder
2020-05-27 18:07 ` Jeff King
2020-05-27 16:47 ` [PATCH 04/12] upload-pack: move filter_capability_requested " Christian Couder
2020-05-27 18:08 ` Jeff King
2020-05-27 16:47 ` [PATCH 05/12] upload-pack: move multi_ack " Christian Couder
2020-05-27 16:47 ` [PATCH 06/12] upload-pack: change multi_ack to an enum Christian Couder
2020-05-27 18:10 ` Jeff King
2020-05-27 16:47 ` [PATCH 07/12] upload-pack: pass upload_pack_data to upload_pack_config() Christian Couder
2020-05-27 18:36 ` Jeff King
2020-05-27 16:47 ` [PATCH 08/12] upload-pack: move keepalive to upload_pack_data Christian Couder
2020-05-27 18:38 ` Jeff King
2020-05-27 16:47 ` [PATCH 09/12] upload-pack: move allow_filter " Christian Couder
2020-05-27 18:39 ` Jeff King
2020-05-27 16:47 ` [PATCH 10/12] upload-pack: move allow_ref_in_want " Christian Couder
2020-05-27 18:41 ` Jeff King
2020-05-27 16:47 ` [PATCH 11/12] upload-pack: move allow_sideband_all " Christian Couder
2020-05-27 16:47 ` [PATCH 12/12] upload-pack: move pack_objects_hook " Christian Couder
2020-05-27 18:55 ` Jeff King
2020-05-27 18:57 ` [PATCH 00/12] upload-pack: use 'struct upload_pack_data' thoroughly, part 2 Jeff King
2020-05-27 20:41 ` Junio C Hamano
2020-06-02 4:16 ` [PATCH v2 00/13] " Christian Couder
2020-06-02 4:16 ` [PATCH v2 01/13] upload-pack: actually use some upload_pack_data bitfields Christian Couder
2020-06-02 4:16 ` [PATCH v2 02/13] upload-pack: annotate upload_pack_data fields Christian Couder
2020-06-02 4:16 ` [PATCH v2 03/13] upload-pack: move static vars to upload_pack_data Christian Couder
2020-06-02 6:59 ` Jeff King
2020-06-02 4:16 ` [PATCH v2 04/13] upload-pack: move use_sideband " Christian Couder
2020-06-02 4:16 ` [PATCH v2 05/13] upload-pack: move filter_capability_requested " Christian Couder
2020-06-02 4:16 ` [PATCH v2 06/13] upload-pack: move multi_ack " Christian Couder
2020-06-02 4:16 ` [PATCH v2 07/13] upload-pack: change multi_ack to an enum Christian Couder
2020-06-02 4:16 ` [PATCH v2 08/13] upload-pack: pass upload_pack_data to upload_pack_config() Christian Couder
2020-06-02 4:16 ` [PATCH v2 09/13] upload-pack: move keepalive to upload_pack_data Christian Couder
2020-06-02 4:16 ` [PATCH v2 10/13] upload-pack: move allow_filter " Christian Couder
2020-06-02 4:16 ` [PATCH v2 11/13] upload-pack: move allow_ref_in_want " Christian Couder
2020-06-02 4:16 ` [PATCH v2 12/13] upload-pack: move allow_sideband_all " Christian Couder
2020-06-02 4:16 ` [PATCH v2 13/13] upload-pack: move pack_objects_hook " Christian Couder
2020-06-02 7:08 ` [PATCH v2 00/13] upload-pack: use 'struct upload_pack_data' thoroughly, part 2 Jeff King
2020-06-02 17:24 ` Junio C Hamano
2020-06-02 19:05 ` [PATCH] fixup! upload-pack: change multi_ack to an enum Jonathan Tan
2020-06-02 19:28 ` Christian Couder
2020-06-04 17:54 ` [PATCH v3 00/13] upload-pack: use 'struct upload_pack_data' thoroughly, part 2 Christian Couder
2020-06-04 17:54 ` [PATCH v3 01/13] upload-pack: actually use some upload_pack_data bitfields Christian Couder
2020-06-04 17:54 ` [PATCH v3 02/13] upload-pack: annotate upload_pack_data fields Christian Couder
2020-06-04 17:54 ` [PATCH v3 03/13] upload-pack: move static vars to upload_pack_data Christian Couder
2020-06-04 17:54 ` [PATCH v3 04/13] upload-pack: move use_sideband " Christian Couder
2020-06-04 17:54 ` [PATCH v3 05/13] upload-pack: move filter_capability_requested " Christian Couder
2020-06-04 17:54 ` [PATCH v3 06/13] upload-pack: move multi_ack " Christian Couder
2020-06-04 17:54 ` [PATCH v3 07/13] upload-pack: change multi_ack to an enum Christian Couder
2020-06-04 17:54 ` [PATCH v3 08/13] upload-pack: pass upload_pack_data to upload_pack_config() Christian Couder
2020-06-04 17:54 ` [PATCH v3 09/13] upload-pack: move keepalive to upload_pack_data Christian Couder
2020-06-04 17:54 ` [PATCH v3 10/13] upload-pack: move allow_filter " Christian Couder
2020-06-04 17:54 ` [PATCH v3 11/13] upload-pack: move allow_ref_in_want " Christian Couder
2020-06-04 17:54 ` [PATCH v3 12/13] upload-pack: move allow_sideband_all " Christian Couder
2020-06-04 17:54 ` [PATCH v3 13/13] upload-pack: move pack_objects_hook " Christian Couder
2020-06-04 18:07 ` [PATCH v3 00/13] upload-pack: use 'struct upload_pack_data' thoroughly, part 2 Junio C Hamano
2020-06-05 10:38 ` Christian Couder
2020-06-11 12:05 ` [PATCH 00/14] upload-pack: use 'struct upload_pack_data' thoroughly, part 3 Christian Couder
2020-06-11 12:05 ` [PATCH 01/14] upload-pack: pass upload_pack_data to send_shallow_list() Christian Couder
2020-06-11 12:05 ` [PATCH 02/14] upload-pack: pass upload_pack_data to deepen() Christian Couder
2020-06-11 12:05 ` [PATCH 03/14] upload-pack: pass upload_pack_data to deepen_by_rev_list() Christian Couder
2020-06-11 12:05 ` [PATCH 04/14] upload-pack: pass upload_pack_data to send_unshallow() Christian Couder
2020-06-11 12:05 ` [PATCH 05/14] upload-pack: move shallow_nr to upload_pack_data Christian Couder
2020-06-11 12:05 ` [PATCH 06/14] upload-pack: move extra_edge_obj " Christian Couder
2020-06-11 12:05 ` [PATCH 07/14] upload-pack: move allow_unadvertised_object_request " Christian Couder
2020-06-11 12:05 ` [PATCH 08/14] upload-pack: change allow_unadvertised_object_request to an enum Christian Couder
2020-06-11 12:05 ` [PATCH 09/14] upload-pack: pass upload_pack_data to process_haves() Christian Couder
2020-06-11 12:05 ` [PATCH 10/14] upload-pack: pass upload_pack_data to send_acks() Christian Couder
2020-06-11 12:05 ` [PATCH 11/14] upload-pack: pass upload_pack_data to ok_to_give_up() Christian Couder
2020-06-11 12:05 ` [PATCH 12/14] upload-pack: pass upload_pack_data to got_oid() Christian Couder
2020-06-11 12:05 ` [PATCH 13/14] upload-pack: move oldest_have to upload_pack_data Christian Couder
2020-06-11 12:05 ` [PATCH 14/14] upload-pack: refactor common code into do_got_oid() Christian Couder
2020-06-11 20:04 ` [PATCH 00/14] upload-pack: use 'struct upload_pack_data' thoroughly, part 3 Jonathan Tan
Reply instructions:
You may reply publicly to this message via plain-text email
using any one of the following methods:
* Save the following mbox file, import it into your mail client,
and reply-to-all from there: mbox
Avoid top-posting and favor interleaved quoting:
https://en.wikipedia.org/wiki/Posting_style#Interleaved_style
List information: http://vger.kernel.org/majordomo-info.html
* Reply using the --to, --cc, and --in-reply-to
switches of git-send-email(1):
git send-email \
--in-reply-to=fac9a9f1-7ee6-5138-9a9e-94e742b69f2c@gmail.com \
--to=stolee@gmail.com \
--cc=chriscool@tuxfamily.org \
--cc=christian.couder@gmail.com \
--cc=dstolee@microsoft.com \
--cc=git@vger.kernel.org \
--cc=gitster@pobox.com \
--cc=jonathantanmy@google.com \
--cc=jrnieder@gmail.com \
--cc=me@ttaylorr.com \
--cc=peff@peff.net \
/path/to/YOUR_REPLY
https://kernel.org/pub/software/scm/git/docs/git-send-email.html
* If your mail client supports setting the In-Reply-To header
via mailto: links, try the mailto: link
Be sure your reply has a Subject: header at the top and a blank line
before the message body.
Code repositories for project(s) associated with this public inbox
https://80x24.org/mirrors/git.git
This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox;
as well as URLs for read-only IMAP folder(s) and NNTP newsgroup(s).