Re: git log - crash and core dump
- From
René Scharfe <rene.scharfe@lsrfire.ath.cx>
- Date
- Apr 16, 2013, 21:10 UTC
- Message-ID
- <516DBE2E.4060201@lsrfire.ath.cx>
- In-Reply-To
- <7v61zml0ow.fsf@alter.siamese.dyndns.org>
First, lest I forget again: Thank you, Ivan, for the very useful bug report!
Am 16.04.2013 21:45, schrieb Junio C Hamano:
Show 40 quoted lines
> René Scharfe <rene.scharfe@lsrfire.ath.cx> 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@gmail.com> Signed-off-by: Rene Scharfe <rene.scharfe@lsrfire.ath.cx> --- 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