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

Re: [PATCH v2 1/3] fetch-pack: return enum from process_acks()

From
Jonathan Nieder <jrnieder@gmail.com>
Date
Apr 28, 2020, 00:53 UTC
Message-ID
<20200428005333.GE36078@google.com>
In-Reply-To
<f0cfbc03c27658e54a73c46570c5153961ed85b6.1588031728.git.jonathantanmy@google.com>
Jonathan Tan wrote:
Show 8 quoted lines
> process_acks() returns 0, 1, or 2, depending on whether "ready" was
> received and if not, whether at least one commit was found to be common.
> Replace these magic numbers with a documented enum.
>
> Signed-off-by: Jonathan Tan <jonathantanmy@google.com>
> ---
>  fetch-pack.c | 35 ++++++++++++++++++++++++++++-------
>  1 file changed, 28 insertions(+), 7 deletions(-)
Yay!
Show 32 quoted lines
> --- a/fetch-pack.c
> +++ b/fetch-pack.c
> @@ -1268,9 +1268,29 @@ static int process_section_header(struct packet_reader *reader,
>  	return ret;
>  }
>  
> -static int process_acks(struct fetch_negotiator *negotiator,
> -			struct packet_reader *reader,
> -			struct oidset *common)
> +enum common_found {
> +	/*
> +	 * No commit was found to be possessed by both the client and the
> +	 * server, and "ready" was not received.
> +	 */
> +	NO_COMMON_FOUND,
> +
> +	/*
> +	 * At least one commit was found to be possessed by both the client and
> +	 * the server, and "ready" was not received.
> +	 */
> +	COMMON_FOUND,
> +
> +	/*
> +	 * "ready" was received, indicating that the server is ready to send
> +	 * the packfile without any further negotiation.
> +	 */
> +	READY
> +};
> +
> +static enum common_found process_acks(struct fetch_negotiator *negotiator,
> +				      struct packet_reader *reader,
> +				      struct oidset *common)
Thanks for the clear comments.
Show 7 quoted lines
>  {
>  	/* received */
>  	int received_ready = 0;
> @@ -1320,7 +1340,8 @@ static int process_acks(struct fetch_negotiator *negotiator,
>  		die(_("expected no other sections to be sent after no 'ready'"));
>  
>  	/* return 0 if no common, 1 if there are common, or 2 if ready */
This comment can be removed now, which is a nice bonus.
Show 23 quoted lines
> -	return received_ready ? 2 : (received_ack ? 1 : 0);
> +	return received_ready ? READY :
> +		(received_ack ? COMMON_FOUND : NO_COMMON_FOUND);
>  }
>  
>  static void receive_shallow_info(struct fetch_pack_args *args,
> @@ -1508,13 +1529,13 @@ static struct ref *do_fetch_pack_v2(struct fetch_pack_args *args,
>  		case FETCH_PROCESS_ACKS:
>  			/* Process ACKs/NAKs */
>  			switch (process_acks(negotiator, &reader, &common)) {
> -			case 2:
> +			case READY:
>  				state = FETCH_GET_PACK;
>  				break;
> -			case 1:
> +			case COMMON_FOUND:
>  				in_vain = 0;
>  				/* fallthrough */
> -			default:
> +			case NO_COMMON_FOUND:
>  				state = FETCH_SEND_REQUEST;
>  				break;
>  			}
With the extraneous comment removed,
Reviewed-by: Jonathan Nieder <jrnieder@gmail.com>
Thanks.
diff --git i/fetch-pack.c w/fetch-pack.c
index 45547a621e6..7b0eef98db5 100644
--- i/fetch-pack.c
+++ w/fetch-pack.c
@@ -1339,7 +1339,6 @@ static enum common_found process_acks(struct fetch_negotiator *negotiator,
 	if (!received_ready && reader->status != PACKET_READ_FLUSH)
 		die(_("expected no other sections to be sent after no 'ready'"));
 
-	/* return 0 if no common, 1 if there are common, or 2 if ready */
 	return received_ready ? READY :
 		(received_ack ? COMMON_FOUND : NO_COMMON_FOUND);
 }
Previous: Jonathan TanNext: Junio C Hamano
Message 12 of 25 in “Protocol v2 in_vain fixes”
  1. 0/2 Protocol v2 in_vain fixesJonathan Tan, Apr 25, 2020
  2. 1/2 fetch-pack: in protocol v2, in_vain only after ACKJonathan Tan, Apr 25, 2020
  3. Junio C HamanoApr 25, 2020
  4. Jonathan NiederApr 26, 2020
  5. Jonathan TanApr 27, 2020
  6. Junio C HamanoApr 27, 2020
  7. 2/2 fetch-pack: in protocol v2, reset in_vain upon ACKJonathan Tan, Apr 25, 2020
  8. Jonathan NiederApr 26, 2020
  9. Jonathan TanApr 27, 2020
  10. 0/3 Protocol v2 in_vain fixesJonathan Tan, Apr 28, 2020
  11. 1/3 fetch-pack: return enum from process_acks()Jonathan Tan, Apr 28, 2020
  12. Jonathan NiederApr 28, 2020
  13. Junio C HamanoApr 28, 2020
  14. Michal SuchánekApr 28, 2020
  15. Junio C HamanoApr 28, 2020
  16. 2/3 fetch-pack: in protocol v2, in_vain only after ACKJonathan Tan, Apr 28, 2020
  17. Jonathan NiederApr 28, 2020
  18. Johannes SchindelinMay 6, 2020
  19. t5500: count objects through stderr, not traceJonathan Tan, May 6, 2020
  20. Junio C HamanoMay 6, 2020
  21. Junio C HamanoMay 6, 2020
  22. Johannes SchindelinMay 7, 2020
  23. Johannes SchindelinOct 13, 2020
  24. 3/3 fetch-pack: in protocol v2, reset in_vain upon ACKJonathan Tan, Apr 28, 2020
  25. Jonathan NiederApr 28, 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.