On 22/05/09 02:03PM, Junio C Hamano wrote: > Plato Kiorpelidis <kioplato@xxxxxxxxx> 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@xxxxxxxxx> > > --- > > 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