Re: git log - crash and core dump

["René Scharfe" <rene.scharfe@lsrfire.ath.cx>]

Am 17.04.2013 08:39, schrieb Jeff King:
> On Tue, Apr 16, 2013 at 10:26:40PM -0700, Junio C Hamano wrote:
>
>> Junio C Hamano <gitster@pobox.com> writes:
>>
>>> René Scharfe <rene.scharfe@lsrfire.ath.cx> writes:
>>>
>>>> How about making split_ident_line() a bit friendlier be letting it
>>>> provide the epoch as default time stamp instead of NULL?
>>>
>>> Two knee-jerk concerns I have without going back to the callers:
>>>
>>>   * Would that "0" ever be given to the approxidate parser, which
>>>     rejects ancient dates in numbers-since-epoch format without @
>>>     prefix?
>>>
>>>   * Does any existing caller use the NULL as a sign to see the input
>>>     was without date and act on that information?
>>
>> I looked at all the callers (there aren't that many), and none of
>> them did "Do this on a person-only ident, and do that on an ident
>> with timestamp".  So for the callers that ignore timestamp, your
>> patch will be a no-op, and for others that assume there is a
>> timestamp, it will turn a crash/segv into output with funny
>> timestamp.
>
> What about sane_ident_split in builtin/commit.c? It explicitly rejects a
> NULL date. The logic in determine_author_info is a little hard to follow
> (it assembles the ident line with fmt_ident and then reparses it), but I
> believe it should be catching a bogus line from "commit -c", or from
> GIT_AUTHOR_DATE in the environment.

Right, so let's keep the NULLs and fix the individual cases.  A quick
"git grep -W -e date_begin -e date_end -e tz_begin -e tz_end" reveals
that there are only the ones we talked about: blame, pretty, commit
and -- of course -- ident.  And only the first two need fixing.

> As a side note, when determine_author_info sees a bogus ident, it
> appears to just silently ignore it, which is probably a bad thing.
> Shouldn't we by complaining?  Or am I mis-reading the code?

The code looks complicated, but I just tried it: fmt_ident() dies if you 
give it an invalid date.

René