From: Jeff King Date: Tue, 01 Dec 2020 10:26:35 GMT Subject: Re: [PATCH 3/6] fixup! reftable: rest of library Message-ID: 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 > > 0-sized arrays are actually not portable. Definitely. > 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