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

[Plato Kiorpelidis <kioplato@gmail.com>]

On 22/05/09 02:03PM, Junio C Hamano wrote:
> 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.

From what I understand, this is a point about why EXIT_FAILURE and EXIT_SUCCESS
are a bad idea in Git's case in general; not specifically in test-tool's case.
The test-tool doesn't use child processes, therefore it doesn't use the macro
WEXITSTATUS. As a result, supposedly, we could use EXIT_FAILURE and EXIT_SUCCESS
constants in this case. However, we don't want to use them in order to stay
consistent throughtout Git's implementation. Is my understanding correct?

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

This improves upon my attempt to be consistent in what we return, by also giving
the option to tell where in the program we exited a failure. I'll adopt this in
the next iteration of these series, v3.

> Thanks.

Thanks,
Plato