Re: [PATCH] do not read beyond end of malloc'd buffer

From: Jim Meyering <jim@meyering.net>
Date: May 21, 2011, 09:42 UTC
Message-ID: <87ei3s75kf.fsf@rho.meyering.net>
In-Reply-To: <7vfwo92p19.fsf@alter.siamese.dyndns.org>

Junio C Hamano wrote:

Show 11 quoted lines

> Jim Meyering <jim@meyering.net> writes:
>> I was surprised to see "git diff --word-diff" output a ton of
>> garbage, and tracked it down to a bug that's triggered when the
>> diff.suppress-blank-empty config option to true and when at least
>> one of the context lines is empty.
>
> Heh, I am not that surprised ;-)
>
> I think the real culprit is a year-old 882749a (diff: add --word-diff
> option that generalizes --color-words, 2010-04-14); it probably shows that
> not many people use diff.s-b-e settings?

That must be the issue: too few people know to use diff.s-b-e. If a few more of us were to set that option by running this command:

    git config --global diff.suppress-blank-empty true

(to suppress emission of trailing blanks on empty context lines) then bugs like this would be exposed more quickly.

...

Show 5 quoted lines

> It is a bit unfortunate that we cannot make this into a test script, as it
> depends on what is on the uninitialized part of the heap, which might
> happen to be a NUL in which case the test would pass.
>
> Running tests under the valgrind mode may catch issues, though.

Right, since reading the trailing NUL byte after the end of the buffer would be detected.

> Thanks. Will queue with this test squashed in.

Thanks for adding the test!