First, lest I forget again: Thank you, Ivan, for the very useful
bug report!
Am 16.04.2013 21:45, schrieb Junio C Hamano:
> René Scharfe <rene.scharfe@xxxxxxxxxxxxxx> writes:
>
>> Does this patch help?
>>
>> pretty.c | 10 ++++++----
>> 1 file changed, 6 insertions(+), 4 deletions(-)
>>
>> diff --git a/pretty.c b/pretty.c
>> index d3a82d2..713eefc 100644
>> --- a/pretty.c
>> +++ b/pretty.c
>> @@ -405,8 +405,8 @@ void pp_user_info(const struct pretty_print_context *pp,
>> const char *mailbuf, *namebuf;
>> size_t namelen, maillen;
>> int max_length = 78; /* per rfc2822 */
>> - unsigned long time;
>> - int tz;
>> + unsigned long time = 0;
>> + int tz = 0;
>>
>> if (pp->fmt == CMIT_FMT_ONELINE)
>> return;
>> @@ -438,8 +438,10 @@ void pp_user_info(const struct pretty_print_context *pp,
>> strbuf_add(&name, namebuf, namelen);
>>
>> namelen = name.len + mail.len + 3; /* ' ' + '<' + '>' */
>> - time = strtoul(ident.date_begin, &date, 10);
>> - tz = strtol(date, NULL, 10);
>> + if (ident.date_begin) {
>> + time = strtoul(ident.date_begin, &date, 10);
>> + tz = strtol(date, NULL, 10);
>> + }
>>
>> if (pp->fmt == CMIT_FMT_EMAIL) {
>> strbuf_addstr(sb, "From: ");
>
> Looks like a sensible change. split_ident_line() decided that the
> given input was mangled and decided there is no valid date (the
> input had <> where the timestamp string was required), so the
> updated code leaves the time/tz unspecified.
We'd need update pretty.c::format_person_part() and
builtin/blame.c::get_ac_line() as well, though.
How about making split_ident_line() a bit friendlier be letting it
provide the epoch as default time stamp instead of NULL? We shouldn't
do that if we'd like to be able to tell a missing/broken time stamp
apart from a commit that was actually made back in 1970 (e.g. an
imported one). Or if we'd like to not show a time stamp in git log
output at all in that case.
-- >8 --
Subject: ident: let split_ident_line() provide a default time stamp
If a commit has a broken time stamp, split_ident_line() sets
date_begin, date_end, tz_begin and tz_end to NULL. Not all callers
are prepared to handle that case and segfault.
Instead of fixing them and having to be careful while implementing
the next caller, provide a string consisting of the number zero as
default value, representing the UNIX epoch. That's the value that
git log showed before it was converted to use split_ident_line().
Reported-by: Ivan Lyapunov <dront78@xxxxxxxxx>
Signed-off-by: Rene Scharfe <rene.scharfe@xxxxxxxxxxxxxx>
---
ident.c | 10 ++++++----
1 file changed, 6 insertions(+), 4 deletions(-)
diff --git a/ident.c b/ident.c
index 1c123e6..ee840f4 100644
--- a/ident.c
+++ b/ident.c
@@ -191,6 +191,8 @@ static void strbuf_addstr_without_crud(struct strbuf *sb, const char *src)
sb->buf[sb->len] = '\0';
}
+static const char zero_string[] = "0";
+
/*
* Reverse of fmt_ident(); given an ident line, split the fields
* to allow the caller to parse it.
@@ -254,10 +256,10 @@ int split_ident_line(struct ident_split *split, const char *line, int len)
return 0;
person_only:
- split->date_begin = NULL;
- split->date_end = NULL;
- split->tz_begin = NULL;
- split->tz_end = NULL;
+ split->date_begin = zero_string;
+ split->date_end = zero_string + strlen(zero_string);
+ split->tz_begin = zero_string;
+ split->tz_end = zero_string + strlen(zero_string);
return 0;
}
--
1.8.2.1
--
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