Re: [PATCH v2 04/15] test-dir-iterator: consistently return EXIT_FAILURE or EXIT_SUCCESS
- From
Junio C Hamano <gitster@pobox.com>
- Date
- May 9, 2022, 21:03 UTC
- Message-ID
- <xmqq35hictaa.fsf@gitster.g>
- In-Reply-To
- <20220509175159.2948802-5-kioplato@gmail.com>
Plato Kiorpelidis <kioplato@gmail.com> writes:
Show 9 quoted lines
> Throughout test-dir-iterator.c we were returning/exiting with either > integers or EXIT_FAILURE. Improve readability and reduce mental load > by being consistent with what test-dir-iterator returns through the > test-tool. Returning mixed constants and integers could indicate that > it matters for some reason e.g. architecture of test-tool and cmd__* > functions. > > EXIT_SUCCESS and EXIT_FAILURE are specified by the C standard. > That makes the code more portable and standardized.
And less portable for the invoking process of "git". The invoking process used to be able to depend ou getting WEXITSTATUS() from our exit status to obtain "1" when we exited with 1; if we start exiting with EXIT_FAILURE, there is no guarantee what non-zero value is used.
So, I am not sure if this is a good direction to go in.
> Signed-off-by: Plato Kiorpelidis <kioplato@gmail.com> > --- > t/helper/test-dir-iterator.c | 6 +++--- > 1 file changed, 3 insertions(+), 3 deletions(-)
Especially given that this is a helper for testing, we may want to return/exit with different non-zero value at different places to make it easier for the test script to tell where in the program we decided to exit a failure. IOW, if we return not EXIT_FAILURE but 2 (or whatever value that is not used elsewhere) in the first hunk, and let the second hunk continue to return 1, then the calling test script can tell which one decided to fail.
As to EXIT_SUCCESS, I do not have a strong opinion against it, but because EXIT_FAILURE is a bad idea, we probably should avoid it for consistency.
Thanks.
Show 24 quoted lines
> diff --git a/t/helper/test-dir-iterator.c b/t/helper/test-dir-iterator.c
> index 659b6bfa81..81e931673e 100644
> --- a/t/helper/test-dir-iterator.c
> +++ b/t/helper/test-dir-iterator.c
> @@ -39,7 +39,7 @@ int cmd__dir_iterator(int argc, const char **argv)
>
> if (!diter) {
> printf("dir_iterator_begin failure: %s\n", error_name(errno));
> - exit(EXIT_FAILURE);
> + return EXIT_FAILURE;
> }
>
> while ((iter_status = dir_iterator_advance(diter)) == ITER_OK) {
> @@ -58,8 +58,8 @@ int cmd__dir_iterator(int argc, const char **argv)
>
> if (iter_status != ITER_DONE) {
> printf("dir_iterator_advance failure\n");
> - return 1;
> + return EXIT_FAILURE;
> }
>
> - return 0;
> + return EXIT_SUCCESS;
> }