On Sun, Oct 28, 2012 at 12:49:55AM +0200, Romain Francoise wrote: > AddressSanitizer (http://clang.llvm.org/docs/AddressSanitizer.html) > complains of a one-byte buffer underflow in parse_name_and_email() while > running the test suite. And indeed, if one of the lines in the mailmap > begins with '<', we dereference the address just before the beginning of > the buffer when looking for whitespace to remove, before checking that > we aren't going too far. > > So reverse the order of the tests to make sure that we don't read > outside the buffer. Thanks, I think your fix is correct. > diff --git a/mailmap.c b/mailmap.c > index 47aa419..ea4b471 100644 > --- a/mailmap.c > +++ b/mailmap.c > @@ -118,7 +118,7 @@ static char *parse_name_and_email(char *buffer, char **name, > while (isspace(*nstart) && nstart < left) > ++nstart; > nend = left-1; > - while (isspace(*nend) && nend > nstart) > + while (nend > nstart && isspace(*nend)) > --nend; The fix confused me for a moment, because the problem is not actually in the loop condition itself; working backwards from "nend > nstart", we will at worst dereference nstart unnecessarily. The real problem is in the "nend = left-1" above, which sets the loop precondition outside the string to be examined. So you could also check for "left == nstart" before the loop even begins. I think your fix (to just make the loop more robust to that precondition) is better, though, as the rest of the code does the right thing with such a value of nend. It looks like t4203 triggers this problem. Curious that valgrind does not find it. I guess since it does not have compiler support, it cannot find out-of-bound errors on stack buffers. Does the rest of the test suite turn up clean with AddressSanitizer? -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