David Reiss <dreiss@xxxxxxxxxxxx> writes: > The author name should never be missing in a valid commit, but > git shouldn't segfault no matter what is in the object database. > > Signed-off-by: David Reiss <dreiss@xxxxxxxxxxxx> > --- > git blame was segfaulting on a repro produced by piping mtn git_export > from the Pidgin repository to git fast-import. This was the most obvious > fix, but I'm not sure if it is the best solution. Thanks. While it is _unusual_ not to have a human readable name, if the commits come from foreign systems (e.g. CVS/SVN), there often are not sufficient information in the source to fabricate names, so we should tolerate them. We might want to also teach fast-import to warn when asked (i.e. when we are feeding from a foreign interface that is designed to read from a source that is capable of recording real names), but we shouldn't prevent it from creating such a commit. > Here's a script that reproduces the segfault. Please make that into a new test in an existing test suite somewhere in t/ directory. I think we probably should prepare ourselves to be fed with even more broken commits, perhaps like this, if we are fixing it.. builtin-blame.c | 13 ++++++++++--- 1 files changed, 10 insertions(+), 3 deletions(-) diff --git a/builtin-blame.c b/builtin-blame.c index d4e25a5..14830a3 100644 --- a/builtin-blame.c +++ b/builtin-blame.c @@ -1305,6 +1305,7 @@ static void get_ac_line(const char *inbuf, const char *what, error_out: /* Ugh */ *tz = "(unknown)"; + strcpy(person, *tz); strcpy(mail, *tz); *time = 0; return; @@ -1314,20 +1315,26 @@ static void get_ac_line(const char *inbuf, const char *what, tmp = person; tmp += len; *tmp = 0; - while (*tmp != ' ') + while (person < tmp && *tmp != ' ') tmp--; + if (tmp == person) + goto error_out; *tz = tmp+1; tzlen = (person+len)-(tmp+1); *tmp = 0; - while (*tmp != ' ') + while (person < tmp && *tmp != ' ') tmp--; + if (tmp == person) + goto error_out; *time = strtoul(tmp, NULL, 10); timepos = tmp; *tmp = 0; - while (*tmp != ' ') + while (person < tmp && *tmp != ' ') tmp--; + if (tmp <= person) + return; mailpos = tmp + 1; *tmp = 0; maillen = timepos - tmp; -- 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