Re: [RFC PATCH] rev-parse: fix segfault with missing --path-format argument

On 2021-05-16 21:53, Junio C Hamano wrote:

As die() is end-user facing, you'd probably want

	die(_("--path-format requires an argument"));

We do have untranslated die() nearby for the same option, which may
want to be cleaned up either in a preliminary patch, or in this same
patch as an unrelated fix "while we are at it".

I would not mind preparing a preliminary patch that cleans up all
untranslated user-facing calls to die(). My editor finds 15 of those in
rev-parse.c, and I think they all qualify.

If you'd rather not touch unrelated code paths I'll instead include it
in v2 as an unrelated fix in the same commit.

The above is certainly worth testing for, but if we ever upgrade the
command line parser of "rev-parse" to be compatible with the parser
based on the parse-options API to allow both "--opt=val" and "--opt
val", it will start to fail for an entirely different reason, namely
"--show-toplevel" will be taken as the argument to "--path-format",
and we'd get "unknown argument to --path-format".  So it might be
prudent to test both, i.e.

	test_must_fail git rev-parse --path-format --show-toplevel &&
	test_must_fail git rev-parse --show-toplevel --path-format

I think I initially went for "--path-format --show-toplevel" because I
was under the assumption that --path-format needs another option it can
modify. It seems that this is not the case, so wouldn't it be simpler
here to do the following instead:

	test_must_fail git rev-parse --path-format

That way we do not have to worry about subsequent changes to other,
unrelated, options.

-- 
Wolfgang