Re: [PATCH 3/6] fixup! reftable: rest of library

From: Jeff King <peff@peff.net>
Date: Dec 1, 2020, 10:26 UTC
Message-ID: <X8YaWySkyRU3DA87@coredump.intra.peff.net>
In-Reply-To: <58f2b0394546e8da2922adcbc38bdb6b53f2b313.1606545878.git.gitgitgadget@gmail.com>

On Sat, Nov 28, 2020 at 06:44:35AM +0000, Johannes Schindelin via GitGitGadget wrote:

> From: Johannes Schindelin <johannes.schindelin@gmx.de>
> 
> 0-sized arrays are actually not portable.

Definitely.

Show 11 quoted lines

>  static void test_sizes_to_segments_empty(void)
>  {
> -	uint64_t sizes[0];
> +	uint64_t sizes[1];
>  
>  	int seglen = 0;
>  	struct segment *segs =
> -		sizes_to_segments(&seglen, sizes, ARRAY_SIZE(sizes));
> +		sizes_to_segments(&seglen, sizes, 0);
>  	EXPECT(seglen == 0);
>  	reftable_free(segs);

I think passing:

  sizes_to_segments(&seglen, NULL, 0);

may make the code more obvious. Unlike system functions like memcpy(), we can be assured of whether our functions avoid looking at a zero-length array (and size_to_segments does follow that rule).

  This function, of course, is nonsense that real code would not do, and
  is just unit-testing sizes_to_segments. I'm not wild in general about
  having a parallel suite of C tests that does not interact with our
  usual tests, but it may be the least bad way to benefit from the
  unit-test coverage that reftable ships with. As a rule, I'd much
  rather see a tool exposing functionality to the command-line, which
  can then be driven independently. I recognize that can end up
  complicated, though.

-Peff