Re: [PATCH v2] fetch-pack: do not reset in_vain on non-novel acks
From: Junio C Hamano <hidden>
Date: 2016-09-23 19:40:41
Jonathan Tan [off-list ref] writes:
I tried looking at creating a helper function to reduce both the size and the nesting level of the loop, but it seems to me that a helper function can't be extracted so easily because the logic is quite intertwined with the rest of the function. For example, the "if (args->stateless_rpc..." block uses 6 variables from the outer scope: args, ack, commit, result_sha1, req_buf, and state_len (and in_vain, but this can be the return value of the function). Expanding it wider would allow us to make some of those 6 local, but also introduce new ones from the outer scope.
Yup, I suspected that much when I wrote the message you are responding to, but was sort-of hoping that you might come up with a more clever way to restructure the code. It is OK to leave it as-is, and let others try making it cleaner ;-). Thanks.
quoted hunk
fetch-pack.c | 11 +++++++++-- 1 file changed, 9 insertions(+), 2 deletions(-)diff --git a/fetch-pack.c b/fetch-pack.c index 85e77af..413937e 100644 --- a/fetch-pack.c +++ b/fetch-pack.c@@ -428,10 +428,17 @@ static int find_common(struct fetch_pack_args *args, const char *hex = sha1_to_hex(result_sha1); packet_buf_write(&req_buf, "have %s\n", hex); state_len = req_buf.len; - } + /* + * Reset in_vain because an ack + * for this commit has not been + * seen. + */ + in_vain = 0; + } else if (!args->stateless_rpc + || ack != ACK_common) + in_vain = 0; mark_common(commit, 0, 1); retval = 0; - in_vain = 0; got_continue = 1; if (ack == ACK_ready) { clear_prio_queue(&rev_list);