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

[Josh Steadmon <steadmon@google.com>]

On 2018.11.13 19:28, SZEDER Gábor wrote:
> 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.