On Tue, Apr 16, 2013 at 10:26:40PM -0700, Junio C Hamano wrote: > Junio C Hamano <gitster@xxxxxxxxx> writes: > > > René Scharfe <rene.scharfe@xxxxxxxxxxxxxx> 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. 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? -Peff -- To unsubscribe from this list: send the line "unsubscribe git" in the body of a message to majordomo@xxxxxxxxxxxxxxx More majordomo info at http://vger.kernel.org/majordomo-info.html