Subject: Re: [PATCH v2 12/13] reftable: rest of library

From: Junio C Hamano <[email-masked]>
Date: Fri, 02 Oct 2020 10:08:58 -0700

Johannes Schindelin <[email-masked]> writes:

> Subject: [PATCH 1/3] fixup??? reftable: rest of library

This is unambiguously good.

> Subject: [PATCH 2/3] fixup??? reftable: rest of library
>
> 0-sized arrays are actually not portable.
> ...
>  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);
>  	assert(seglen == 0);
>  	reftable_free(segs);

Question to Han-Wen.

It is unclear what this test wants to test.  Do we even need sizes[]
array if we know we are passing a hardcoded 0?  IOW, I would
understand if the test were

	sizes_to_segments(&seglen, NULL, 0);

to ensure that sizes_to_segments do not even attempt to look at sizes[]
array when the number of elements is 0.

> Subject: [PATCH 3/3] fixup??? reftable: rest of library
>
> Avoid using `getopt()`: it might be POSIX, but Git's audience is much
> larger than POSIX. MSVC, for example, does not support `getopt()`.

Either that, or we could use parse-options().  I do not care either
way, as this seems to be purely for debugging?