Re: [PATCH v2 6/9] connect: teach client to recognize v1 server response
- From
Junio C Hamano <gitster@pobox.com>
- Date
- Sep 27, 2017, 05:29 UTC
- Message-ID
- <xmqqpoac676l.fsf@gitster.mtv.corp.google.com>
- In-Reply-To
- <20170926235627.79606-7-bmwill@google.com>
Brandon Williams <bmwill@google.com> writes:
Show 12 quoted lines
> +/* Returns 1 if packet_buffer is a protocol version pkt-line, 0 otherwise. */
> +static int process_protocol_version(void)
> +{
> + switch (determine_protocol_version_client(packet_buffer)) {
> + case protocol_v1:
> + return 1;
> + case protocol_v0:
> + return 0;
> + default:
> + die("server is speaking an unknown protocol");
> + }
> +}For the purpose of "technology demonstration" v1 protocol, it is OK to discard the result of "determine_pvc()" like the above code, but in a real application, we would do a bit more than just ignoring an extra "version #" packet that appears at the beginning, no?
It would be sensible to design how the result of determien_pvc() call is propagated to the remainder of the program in this patch and implement it. Perhaps add a new global (like server_capabilities already is) and store the value there, or something? Or pass a pointer to enum protocol_version as a return-location parameter to this helper function so that the process_capabilities() can pass a pointer to its local variable?
Show 23 quoted lines
> static void process_capabilities(int *len)
> {
> @@ -224,12 +239,19 @@ struct ref **get_remote_heads(int in, char *src_buf, size_t src_len,
> */
> int responded = 0;
> int len;
> - int state = EXPECTING_FIRST_REF;
> + int state = EXPECTING_PROTOCOL_VERSION;
>
> *list = NULL;
>
> while ((len = read_remote_ref(in, &src_buf, &src_len, &responded))) {
> switch (state) {
> + case EXPECTING_PROTOCOL_VERSION:
> + if (process_protocol_version()) {
> + state = EXPECTING_FIRST_REF;
> + break;
> + }
> + state = EXPECTING_FIRST_REF;
> + /* fallthrough */
> case EXPECTING_FIRST_REF:
> process_capabilities(&len);
> if (process_dummy_ref()) {