fetch-pack: directly end negotiation if ACK ready

When "ACK %s ready" is received, find_common() clears rev_list in an attempt to stop further "have" lines from being sent [1]. It is much more readable to explicitly break from the loop instead. So explicitly break from the loop, and make the clearing of the rev_list happen unconditionally. [1] The rationale is further described in the originating commit f2cba9299b ("fetch-pack: Finish negotation if remote replies "ACK %s ready"", 2011-03-14). Signed-off-by: Jonathan Tan <jonathantanmy@google.com> Signed-off-by: Junio C Hamano <gitster@pobox.com>

Jonathan Tan committed Jun 14, 2018 at 15:54 UTC 21bcf6e429e01961d38bc639b2dc4a6f64b8fdcf
1 file changed +5 -4
fetch-pack.c
+5 -4
@@ -517,10 +517,8 @@ static int find_common(struct fetch_pack_args *args,
517 mark_common(commit, 0, 1);
518 retval = 0;
519 got_continue = 1;
520 - if (ack == ACK_ready) {
521 - clear_prio_queue(&rev_list);
520 + if (ack == ACK_ready)
521 got_ready = 1;
523 - }
522 break;
523 }
524 }
@@ -530,6 +528,8 @@ static int find_common(struct fetch_pack_args *args,
528 print_verbose(args, _("giving up"));
529 break; /* give up */
530 }
531 + if (got_ready)
532 + break;
533 }
534 }
535 done:
@@ -1096,6 +1096,7 @@ static struct ref *do_fetch_pack(struct fetch_pack_args *args,
1096 die(_("git fetch-pack: fetch failed."));
1097
1098 all_done:
1099 + clear_prio_queue(&rev_list);
1100 return ref;
1101 }
1102
@@ -1300,7 +1301,6 @@ static int process_acks(struct packet_reader *reader, struct oidset *common)
1301 }
1302
1303 if (!strcmp(reader->line, "ready")) {
1303 - clear_prio_queue(&rev_list);
1304 received_ready = 1;
1305 continue;
1306 }
@@ -1441,6 +1441,7 @@ static struct ref *do_fetch_pack_v2(struct fetch_pack_args *args,
1441 }
1442 }
1443
1444 + clear_prio_queue(&rev_list);
1445 oidset_clear(&common);
1446 return ref;
1447 }