Re: [PATCH v2 04/15] test-dir-iterator: consistently return EXIT_FAILURE or EXIT_SUCCESS

[Junio C Hamano <gitster@pobox.com>]

Plato Kiorpelidis <kioplato@gmail.com> writes:

> 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.

> 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;
>  }