Re: git log - crash and core dump

From: Jeff King <peff@peff.net>
Date: Apr 17, 2013, 18:02 UTC
Message-ID: <20130417180248.GA1839@sigill.intra.peff.net>
In-Reply-To: <516EE300.7020200@lsrfire.ath.cx>

On Wed, Apr 17, 2013 at 07:59:28PM +0200, René Scharfe wrote:

Show 10 quoted lines

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

Yes, that matches the list I came up with in February.

I think we also need to do something about "git cat-file -p", which does not use the split_ident_line parser (but has its own problems with the home-grown parser).

Show 6 quoted lines

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

It does seem like determine_author_info can be greatly simplified, but I haven't looked all that closely.

-Peff