Erik Bray <erik.m.bray@xxxxxxxxx> writes: > I tracked the issue to refs/files-backend.c in show_one_reflog_ent : > > https://github.com/git/git/blob/11529ecec914d2f0d7575e6d443c2d5a6ff75424/refs/files-backend.c#L2923 > > in which > > !(timestamp = strtoul(email_end + 2, &message, 10)) || > > implies an invalid reflog entry. Why should 0 be treated as an > invalid timestamp (even if it's unlikely outside of corner cases)? Thanks for a report. I think this dates back to 883d60fa (Sanitize for_each_reflog_ent(), 2007-01-08): commit 883d60fa97c6397450fb129634054e0a6101baac Author: Johannes Schindelin <Johannes.Schindelin@xxxxxx> Date: Mon Jan 8 01:59:54 2007 +0100 Sanitize for_each_reflog_ent() It used to ignore the return value of the helper function; now, it expects it to return 0, and stops iteration upon non-zero return values; this value is then passed on as the return value of for_each_reflog_ent(). Further, it makes no sense to force the parsing upon the helper functions; for_each_reflog_ent() now calls the helper function with old and new sha1, the email, the timestamp & timezone, and the message. Signed-off-by: Johannes Schindelin <johannes.schindelin@xxxxxx> Signed-off-by: Junio C Hamano <junkio@xxxxxxx> I do not see a corresponding "timestamp must be non-zero" check in the lines removed by that commit. I suspect that there was some confusion as to how strtoul() signals an error condition when the commit was written, or something. I do not think existing implementations of Git supports timestamps before the epoch (the timestamp on the common object headers are read into unsigned long variables and the code is not prepared to see anything negative there), but if anything the code should be detecting errors from strtoul() properly, i.e. if a timestamp is way long into the future and does not fit in "unsigned long", we should detect it. Checking the value against ULONG_MAX and errno==ERANGE would be an improvement. It may be debatable if we should silently ignore an entry with an invalid timestamp, but that is a separate issue. refs.c | 3 ++- 1 file changed, 2 insertions(+), 1 deletion(-) diff --git a/refs.c b/refs.c index 4e15f60..ff24184 100644 --- a/refs.c +++ b/refs.c @@ -3701,7 +3701,8 @@ static int show_one_reflog_ent(struct strbuf *sb, each_reflog_ent_fn fn, void *c get_sha1_hex(sb->buf + 41, nsha1) || sb->buf[81] != ' ' || !(email_end = strchr(sb->buf + 82, '>')) || email_end[1] != ' ' || - !(timestamp = strtoul(email_end + 2, &message, 10)) || + ((timestamp = strtoul(email_end + 2, &message, 10)) == ULONG_MAX && + errno == ERANGE) || !message || message[0] != ' ' || (message[1] != '+' && message[1] != '-') || !isdigit(message[2]) || !isdigit(message[3]) || -- 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