Re: [PATCH 5/6] commit-graph: implement file format version 2

From: Ævar Arnfjörð Bjarmason <avarab@gmail.com>
Date: Mar 21, 2019, 09:21 UTC
Message-ID: <87tvfwd1l8.fsf@evledraar.gmail.com>
In-Reply-To: <c55e0a738c79c30affcf2cb68564c01c36c66dff.1548280753.git.gitgitgadget@gmail.com>

On Wed, Jan 23 2019, Derrick Stolee via GitGitGadget wrote:

Show 9 quoted lines

>  	graph_version = *(unsigned char*)(data + 4);
> -	if (graph_version != 1) {
> -		error(_("graph version %X does not match version %X"),
> -		      graph_version, 1);
> +	if (!graph_version || graph_version > 2) {
> +		error(_("unsupported graph version %X"),
> +		      graph_version);
>  		goto cleanup_fail;
>  	}

Just noticed this while writing https://public-inbox.org/git/87va0cd1zp.fsf@evledraar.gmail.com/ i.e. to resolve the conflict with my commit-graph segfault fixing series.

This really should be something like:

	/* earlier */
	#define GRAPH_MAX_VERSION 2

	if (!graph_version || graph_version > GRAPH_MAX_VERSION) {
		error(_("commit-graph unsupported graph version %X, we support up to %X"),
			graph_version, GRAPH_MAX_VERSION);

Also, I'm confused as to what these patches are based on. Your is doing "!= 1", but on "master" and ever since 2a2e32bdc5 ("commit-graph: implement git commit-graph read", 2018-04-10) this has been the macro "GRAPH_VERSION" instead of "1".