Re: [PATCH] rev-parse: Detect missing opt-spec

From: Junio C Hamano <gitster@pobox.com>
Date: Sep 2, 2022, 16:27 UTC
Message-ID: <xmqq5yi5aghf.fsf@gitster.g>
In-Reply-To: <20220902064727.GA3606@szeder.dev>

SZEDER Gábor <szeder.dev@gmail.com> writes:

Show 15 quoted lines

> On Fri, Sep 02, 2022 at 07:06:21AM +0200, Øystein Walle wrote:
>> If a line in parseopts's input starts with one of the flag characters it
>> is erroneously parsed as a opt-spec where the short name of the option
>> is the flag character itself and the long name is after the end of the
>> string. This makes Git want to allocate SIZE_MAX bytes of memory at this
>> line:
>> 
>>     o->long_name = xmemdupz(sb.buf + 2, s - sb.buf - 2);
>> 
>> Since s and sb.buf are equal the second argument is -2 (except unsigned)
>> and xmemdupz allocates len + 1 bytes, ie. -1 meaning SIZE_MAX.
>
> I suspect (but didn't actually check) that this bug was added in
> 2d893dff4c (rev-parse --parseopt: allow [*=?!] in argument hints,
> 2015-07-14).

Good thing to add to the proposed log message.  Thanks.

Also, Øystein "Detect" -> "detect" on the title (you can see the convention in the output from "git shortlog --no-merges").

Show 6 quoted lines

>>  		if (!s)
>>  			s = help;
>>  
>> +		if (s == sb.buf)
>> +			die(_("Missing opt-spec before option flags"));
>> +

OK.

Show 8 quoted lines

>> +test_expect_success 'test --parseopt invalid opt-spec' '
>> +	test_write_lines x -- "=, x" >spec &&
>> +	echo "fatal: Missing opt-spec before option flags" >expect &&
>> +	test_must_fail git rev-parse --parseopt -- >out <spec >actual 2>&1 &&
>
> When checking an error message please don't look for it on standard
> output; i.e. the redirection at the end should be '2>actual', or
> perheps even better '2>err'.

Again, very good point.

Thanks.