Re: [PATCH v3 1/1] protocol: advertise multiple supported versions

2018-11-16 Thread Josh Steadmon
On 2018.11.16 11:45, Junio C Hamano wrote: > Josh Steadmon writes: > > >> What I was alludding to was a lot simpler, though. An advert string > >> "version=0:version=1" from a client that prefers version 0 won't be > >> !strcmp("version=0", advert) but seeing many strcmp() in the patch > >>

Re: [PATCH v3 1/1] protocol: advertise multiple supported versions

2018-11-15 Thread Junio C Hamano
Josh Steadmon writes: >> What I was alludding to was a lot simpler, though. An advert string >> "version=0:version=1" from a client that prefers version 0 won't be >> !strcmp("version=0", advert) but seeing many strcmp() in the patch >> made me wonder. > > Ah I see. The strcmp()s against

Re: [PATCH v3 1/1] protocol: advertise multiple supported versions

2018-11-14 Thread Josh Steadmon
On 2018.11.14 11:38, Junio C Hamano wrote: > Josh Steadmon writes: > > > On 2018.11.13 13:01, Junio C Hamano wrote: > >> I am wondering if the code added by this patch outside this > >> function, with if (strcmp(client_ad.buf, "version=0") sprinkled all > >> over the place, works sensibly when

Re: [PATCH v3 1/1] protocol: advertise multiple supported versions

2018-11-14 Thread SZEDER Gábor
On Wed, Nov 14, 2018 at 11:44:51AM +0900, Junio C Hamano wrote: > SZEDER Gábor writes: > > >> + if (tmp_allowed_versions[0] != config_version) > >> + for (int i = 1; i < nr_allowed_versions; i++) > > > > We don't do C99 yet, thus the declaration of a loop variable like this > > is not

Re: [PATCH v3 1/1] protocol: advertise multiple supported versions

2018-11-13 Thread Junio C Hamano
SZEDER Gábor writes: >> +if (tmp_allowed_versions[0] != config_version) >> +for (int i = 1; i < nr_allowed_versions; i++) > > We don't do C99 yet, thus the declaration of a loop variable like this > is not allowed and triggers compiler errors. I thought we did a weather-balloon

Re: [PATCH v3 1/1] protocol: advertise multiple supported versions

2018-11-13 Thread Junio C Hamano
Josh Steadmon writes: > On 2018.11.13 13:01, Junio C Hamano wrote: >> stead...@google.com writes: >> >> > Currently the client advertises that it supports the wire protocol >> > version set in the protocol.version config. However, not all services >> > support the same set of protocol versions.

Re: Re* [PATCH v3 1/1] protocol: advertise multiple supported versions

2018-11-13 Thread Junio C Hamano
Stefan Beller writes: > "Nobody reads documentation any more, we have too much of it." Maybe, but spelling it out and writing it down, you can point at it (or hope that other people point at it without making a bad or incorrect rephrase). > I would have hoped to have a .cocci patch in the bad

Re: [PATCH v3 1/1] protocol: advertise multiple supported versions

2018-11-13 Thread SZEDER Gábor
On Tue, Nov 13, 2018 at 03:03:47PM -0800, Josh Steadmon wrote: > > > + for (int i = 1; i < nr_allowed_versions; i++) > > > > We don't do C99 yet, thus the declaration of a loop variable like this > > is not allowed and triggers compiler errors. > Sorry about that. Will fix in v4. Out of

Re: [PATCH v3 1/1] protocol: advertise multiple supported versions

2018-11-13 Thread Josh Steadmon
On 2018.11.13 19:28, SZEDER Gábor wrote: > On Mon, Nov 12, 2018 at 01:49:05PM -0800, stead...@google.com wrote: > > > diff --git a/protocol.c b/protocol.c > > index 5e636785d1..54d2ab991b 100644 > > --- a/protocol.c > > +++ b/protocol.c > > > +void

Re: [PATCH v3 1/1] protocol: advertise multiple supported versions

2018-11-13 Thread Josh Steadmon
On 2018.11.13 13:01, Junio C Hamano wrote: > stead...@google.com writes: > > > Currently the client advertises that it supports the wire protocol > > version set in the protocol.version config. However, not all services > > support the same set of protocol versions. When connecting to > >

Re: Re* [PATCH v3 1/1] protocol: advertise multiple supported versions

2018-11-13 Thread Stefan Beller
On Mon, Nov 12, 2018 at 7:24 PM Junio C Hamano wrote: > > Stefan Beller writes: > > >> + if (have_advertised_versions_already) > >> + BUG(_("attempting to register an allowed protocol version > >> after advertisement")); > > > > If this is a real BUG (due to wrong program

Re: [PATCH v3 1/1] protocol: advertise multiple supported versions

2018-11-13 Thread SZEDER Gábor
On Mon, Nov 12, 2018 at 01:49:05PM -0800, stead...@google.com wrote: > diff --git a/protocol.c b/protocol.c > index 5e636785d1..54d2ab991b 100644 > --- a/protocol.c > +++ b/protocol.c > +void get_client_protocol_version_advertisement(struct strbuf *advert) > +{ > + int tmp_nr =

Re: [PATCH v3 1/1] protocol: advertise multiple supported versions

2018-11-13 Thread Junio C Hamano
stead...@google.com writes: > + if (tmp_allowed_versions[0] != config_version) > + for (int i = 1; i < nr_allowed_versions; i++) > + if (tmp_allowed_versions[i] == config_version) { > + enum protocol_version swap = > +

Re: [PATCH v3 1/1] protocol: advertise multiple supported versions

2018-11-12 Thread Junio C Hamano
stead...@google.com writes: > Currently the client advertises that it supports the wire protocol > version set in the protocol.version config. However, not all services > support the same set of protocol versions. When connecting to > git-receive-pack, the client automatically downgrades to v0 if

Re* [PATCH v3 1/1] protocol: advertise multiple supported versions

2018-11-12 Thread Junio C Hamano
Stefan Beller writes: >> + if (have_advertised_versions_already) >> + BUG(_("attempting to register an allowed protocol version >> after advertisement")); > > If this is a real BUG (due to wrong program flow) instead of bad user input, > we would not want to burden the

Re: [PATCH v3 1/1] protocol: advertise multiple supported versions

2018-11-12 Thread Stefan Beller
On Mon, Nov 12, 2018 at 1:49 PM wrote: > > Currently the client advertises that it supports the wire protocol > version set in the protocol.version config. However, not all services > support the same set of protocol versions. When connecting to > git-receive-pack, the client automatically

[PATCH v3 1/1] protocol: advertise multiple supported versions

2018-11-12 Thread steadmon
Currently the client advertises that it supports the wire protocol version set in the protocol.version config. However, not all services support the same set of protocol versions. When connecting to git-receive-pack, the client automatically downgrades to v0 if config.protocol is set to v2, but