Re: [PATCH v3 1/1] protocol: advertise multiple supported versions
- From
Josh Steadmon <steadmon@google.com>
- Date
- Nov 13, 2018, 23:03 UTC
- Message-ID
- <20181113230347.GH126896@google.com>
- In-Reply-To
- <20181113182802.GG30222@szeder.dev>
On 2018.11.13 19:28, SZEDER Gábor wrote:
Show 54 quoted lines
> On Mon, Nov 12, 2018 at 01:49:05PM -0800, steadmon@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 = nr_allowed_versions;
> > + enum protocol_version *tmp_allowed_versions, config_version;
> > + strbuf_reset(advert);
> > +
> > + have_advertised_versions_already = 1;
> > +
> > + config_version = get_protocol_version_config();
> > + if (config_version == protocol_v0) {
> > + strbuf_addstr(advert, "version=0");
> > + return;
> > + }
> > +
> > + if (tmp_nr > 0) {
> > + ALLOC_ARRAY(tmp_allowed_versions, tmp_nr);
> > + copy_array(tmp_allowed_versions, allowed_versions, tmp_nr,
> > + sizeof(enum protocol_version));
> > + } else {
> > + ALLOC_ARRAY(tmp_allowed_versions, 1);
> > + tmp_nr = 1;
> > + tmp_allowed_versions[0] = config_version;
> > + }
> > +
> > + 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.
>
> > + if (tmp_allowed_versions[i] == config_version) {
> > + enum protocol_version swap =
> > + tmp_allowed_versions[0];
> > + tmp_allowed_versions[0] =
> > + tmp_allowed_versions[i];
> > + tmp_allowed_versions[i] = swap;
> > + }
> > +
> > + strbuf_addf(advert, "version=%s",
> > + format_protocol_version(tmp_allowed_versions[0]));
> > + for (int i = 1; i < tmp_nr; i++)
>
> Likewise.
>
> > + strbuf_addf(advert, ":version=%s",
> > + format_protocol_version(tmp_allowed_versions[i]));
> > +}Sorry about that. Will fix in v4. Out of curiousity, do you have a config.mak snippet that will make these into errors? I played around with adding combinations of -ansi, -std=c89, and -pedantic to CFLAGS, but I couldn't get anything that detect the problem without also breaking on other parts of the build.