Phillip Wood <phillip.wood123@xxxxxxxxx> writes: > Hi Patrick > > On 16/08/2024 08:04, Patrick Steinhardt wrote: >> diff --git a/t/unit-tests/unit-test.c b/t/unit-tests/unit-test.c >> index 32a81299e9..82b7635e6a 100644 >> --- a/t/unit-tests/unit-test.c >> +++ b/t/unit-tests/unit-test.c >> @@ -6,10 +6,9 @@ int cmd_main(int argc, const char **argv) >> int ret; >> /* Append the "-t" flag such that the tests generate TAP >> output. */ >> - ALLOC_ARRAY(argv_copy, argc + 2); >> + ALLOC_ARRAY(argv_copy, argc + 1); >> COPY_ARRAY(argv_copy, argv, argc); >> argv_copy[argc++] = "-t"; >> - argv_copy[argc] = NULL; > > I'm confused by this - it looks like it belongs in the previous patch > but why are we deleting the line that adds the terminating NULL to > argv in the first place? I agree with you that it belongs to the previous step. clar_test(), unlike the usual convention used by main(), works solely with argc to tell where the list of argument ends, without requiring argv[] to be an NULL terminated array. I do not know if clar _promises_ that as the calling convention to its users, but if it does so, and if we want to take advantage of that promise and leave the terminating NULL out of argv[], we should do so from the beginning. A NULL-terminated argv[], as long as we make sure argc only counts the real arguments without counting that terminating NULL, would not harm the clar_test(argc, argv) call, so I actually do not mind the way this caller was originally written, with terminating NULL. It looks a lot more familiar, even though we may be wasting memory at the end of argv[] array.
