Re: [PATCH v3 10/35] protocol: introduce enum protocol_version value protocol_v2

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

 



On 02/26, Jonathan Nieder wrote:
> Hi,
> 
> Brandon Williams wrote:
> 
> > Introduce protocol_v2, a new value for 'enum protocol_version'.
> > Subsequent patches will fill in the implementation of protocol_v2.
> >
> > Signed-off-by: Brandon Williams <bmwill@xxxxxxxxxx>
> > ---
> 
> Yay!
> 
> [...]
> > +++ b/builtin/fetch-pack.c
> > @@ -201,6 +201,9 @@ int cmd_fetch_pack(int argc, const char **argv, const char *prefix)
> >  			   PACKET_READ_GENTLE_ON_EOF);
> >  
> >  	switch (discover_version(&reader)) {
> > +	case protocol_v2:
> > +		die("support for protocol v2 not implemented yet");
> > +		break;
> 
> This code goes away in a later patch, so no need to do anything about
> this, but the 'break' is redundant after the 'die'.

I'll fix that.

> 
> [...]
> > --- a/builtin/receive-pack.c
> > +++ b/builtin/receive-pack.c
> > @@ -1963,6 +1963,12 @@ int cmd_receive_pack(int argc, const char **argv, const char *prefix)
> >  		unpack_limit = receive_unpack_limit;
> >  
> >  	switch (determine_protocol_version_server()) {
> > +	case protocol_v2:
> > +		/*
> > +		 * push support for protocol v2 has not been implemented yet,
> > +		 * so ignore the request to use v2 and fallback to using v0.
> > +		 */
> > +		break;
> 
> As you mentioned in the cover letter, it's probably worth doing the
> same fallback on the client side (send-pack), too.
> 
> Otherwise when this client talks to a new-enough server, it would
> request protocol v2 and then get confused when the server responds
> with the protocol v2 it requested.

Some patches later on ensure this.

> 
> Thanks,
> Jonathan

-- 
Brandon Williams



[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