On Fri, Aug 16, 2024 at 09:11:36AM -0700, Junio C Hamano wrote: > 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. Agreed. > 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. Okay. It's not necessary, but I don't mind it much, either. Patrick
