Re: [PATCH v3 07/35] connect: convert get_remote_heads to use struct packet_reader

[Date Prev][Date Next][Thread Prev][Thread Next][Date Index][Thread Index]

 



On Tue, Feb 6, 2018 at 5:12 PM, Brandon Williams <bmwill@xxxxxxxxxx> wrote:

> @@ -56,6 +62,41 @@ static void die_initial_contact(int unexpected)
>                       "and the repository exists."));
>  }
>
> +static enum protocol_version discover_version(struct packet_reader *reader)
> +{
> +       enum protocol_version version = protocol_unknown_version;
> +
> +       /*
> +        * Peek the first line of the server's response to
> +        * determine the protocol version the server is speaking.
> +        */
> +       switch (packet_reader_peek(reader)) {
> +       case PACKET_READ_EOF:
> +               die_initial_contact(0);
> +       case PACKET_READ_FLUSH:
> +       case PACKET_READ_DELIM:
> +               version = protocol_v0;
> +               break;
> +       case PACKET_READ_NORMAL:
> +               version = determine_protocol_version_client(reader->line);
> +               break;
> +       }
> +
> +       /* Maybe process capabilities here, at least for v2 */

We do not (yet) react to v2, so this comment only makes
sense after a later patch? If so please include it later,
as this is confusing for now.


> +       switch (version) {
> +       case protocol_v1:
> +               /* Read the peeked version line */
> +               packet_reader_read(reader);

I wonder if we want to assign version to v0 here,
as now all v1 is done and we could treat the remaining
communication as a v0. Not sure if that helps with some
switch/cases, but as we'd give all cases to have the compiler
not yell at us, this would be no big deal. So I guess we can keep
it v1.

With or without the comment nit, this patch is
Reviewed-by: Stefan Beller <sbeller@xxxxxxxxxx>



[Index of Archives]     [Linux Kernel Development]     [Gcc Help]     [IETF Annouce]     [DCCP]     [Netdev]     [Networking]     [Security]     [V4L]     [Bugtraq]     [Yosemite]     [MIPS Linux]     [ARM Linux]     [Linux Security]     [Linux RAID]     [Linux SCSI]     [Fedora Users]

  Powered by Linux