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

[Date Prev][Date Next][Thread Prev][Thread Next][Date Index][Thread Index]

 



Wolfgang Müller <wolf@oriole.systems> writes:

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

I am puzzled by the last paragraph.  Somebody who does not want to
see "unrelated" codepaths touched would appreciate if a commit that
fixes this segfault does not touch them at the same time.

In any case, I now counted existing die() messages in this file, and
among 15 of them, only 1 is marked with _(...).  I think that it
is the best to apply the patch as-is (without _(...)), adding one
untranslated message to the file.

Then, on top of this change, the 15 untranslated messages can be
marked with _(...) a separate commit (and it does not even have to
be done by you).

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

That's good, too.  Simple.

Thanks.





[Index of Archives]     [Linux Kernel Development]     [Gcc Help]     [IETF Annouce]     [DCCP]     [Netdev]     [Networking]     [Security]     [V4L]     [Bugtraq]     [Yosemite]     [MIPS Linux]     [ARM Linux]     [Linux Security]     [Linux RAID]     [Linux SCSI]     [Fedora Users]

  Powered by Linux