git/list[1] front-page[2] threads[3] people[4] search[5] about
 

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.

Previous: SZEDER GáborNext: SZEDER Gábor
Message 33 of 52 in “Limit client version advertisements”
  1. 0/1 Limit client version advertisementsJosh Steadmon, Oct 2, 2018
  2. 1/1 protocol: limit max protocol version per serviceJosh Steadmon, Oct 2, 2018
  3. Stefan BellerOct 2, 2018
  4. Josh SteadmonOct 3, 2018
  5. Stefan BellerOct 3, 2018
  6. Josh SteadmonOct 5, 2018
  7. Stefan BellerOct 5, 2018
  8. Josh SteadmonOct 10, 2018
  9. Jonathan NiederOct 12, 2018
  10. Jonathan NiederOct 12, 2018
  11. Josh SteadmonOct 12, 2018
  12. Jonathan NiederOct 12, 2018
  13. Junio C HamanoOct 13, 2018
  14. 0/1 Advertise multiple supported proto versionssteadmon@google.com, Oct 12, 2018
  15. 1/1 protocol: advertise multiple supported versionssteadmon@google.com, Oct 12, 2018
  16. Stefan BellerOct 12, 2018
  17. Josh SteadmonOct 22, 2018
  18. Stefan BellerOct 23, 2018
  19. 0/1 Advertise multiple supported proto versionssteadmon@google.com, Nov 12, 2018
  20. 1/1 protocol: advertise multiple supported versionssteadmon@google.com, Nov 12, 2018
  21. Stefan BellerNov 12, 2018
  22. Re* [PATCH v3 1/1] protocol: advertise multiple supported versionsJunio C Hamano, Nov 13, 2018
  23. Stefan BellerNov 13, 2018
  24. Junio C HamanoNov 14, 2018
  25. Junio C HamanoNov 13, 2018
  26. Josh SteadmonNov 13, 2018
  27. Junio C HamanoNov 14, 2018
  28. Josh SteadmonNov 14, 2018
  29. Junio C HamanoNov 16, 2018
  30. Josh SteadmonNov 16, 2018
  31. Junio C HamanoNov 13, 2018
  32. SZEDER GáborNov 13, 2018
  33. Josh SteadmonNov 13, 2018
  34. SZEDER GáborNov 14, 2018
  35. Junio C HamanoNov 14, 2018
  36. SZEDER GáborNov 14, 2018
  37. 0/1 Advertise multiple supported proto versionsJosh Steadmon, Nov 14, 2018
  38. 1/1 protocol: advertise multiple supported versionsJosh Steadmon, Nov 14, 2018
  39. Junio C HamanoNov 14, 2018
  40. Josh SteadmonNov 14, 2018
  41. Junio C HamanoNov 16, 2018
  42. 0/1 Advertise multiple supported proto versionsJosh Steadmon, Nov 16, 2018
  43. 1/1 protocol: advertise multiple supported versionsJosh Steadmon, Nov 16, 2018
  44. Ævar Arnfjörð BjarmasonDec 14, 2018
  45. Josh SteadmonDec 14, 2018
  46. Ævar Arnfjörð BjarmasonDec 14, 2018
  47. Josh SteadmonDec 18, 2018
  48. 0/1 Advertise multiple supported proto versionsJosh Steadmon, Dec 20, 2018
  49. 1/1 protocol: advertise multiple supported versionsJosh Steadmon, Dec 20, 2018
  50. Jonathan TanFeb 5, 2019
  51. Josh SteadmonFeb 7, 2019
  52. Jonathan TanFeb 11, 2019

Read the whole thread, see it on lore, or plain text.

$ cat FOOTERMessages come from the public archive at lore.kernel.org/git, fetched every hour. The front page is chosen and written each morning by an AI editor and can be wrong; the threads themselves are the record. About and API. For agents: an MCP server at https://gitlist.dev/mcp, and any thread, story or person page as Markdown by adding .md to its URL (or sending Accept: text/markdown). Details in /llms.txt.