Re: [PATCH] builtin/blame.c::prepare_lines: fix allocation size of sb->lineno

From: David Kastrup <dak@gnu.org>
Date: Feb 8, 2014, 09:49 UTC
Message-ID: <87lhxmc4sr.fsf@fencepost.gnu.org>
In-Reply-To: <1391851166-10393-1-git-send-email-dak@gnu.org>

David Kastrup <dak@gnu.org> writes:

Show 23 quoted lines

> If we are calling xrealloc on every single line, the least we can do
> is get the right allocation size.
>
> Signed-off-by: David Kastrup <dak@gnu.org>
> ---
> This should be less contentious than the patch in
> <URL:http://permalink.gmane.org/gmane.comp.version-control.git/241561>,
> Message-ID: <1391550392-17118-1-git-send-email-dak@gnu.org> as it
> makes no stylistic decisions whatsoever and only fixes a clear bug.
>
> builtin/blame.c | 4 ++--
>  1 file changed, 2 insertions(+), 2 deletions(-)
>
> diff --git a/builtin/blame.c b/builtin/blame.c
> index e44a6bb..29eb31c 100644
> --- a/builtin/blame.c
> +++ b/builtin/blame.c
> @@ -1779,7 +1779,7 @@ static int prepare_lines(struct scoreboard *sb)
>  	while (len--) {
>  		if (bol) {
>  			sb->lineno = xrealloc(sb->lineno,
> -					      sizeof(int *) * (num + 1));
> +					      sizeof(int) * (num + 1));

But please note that since sb->lineno originally comes from a zeroed memory area and is passed to xrealloc, this requires that after

int *p; memset(&p, 0, sizeof(p));

the equivalence

((void *)p == NULL)

will hold. While this is true on most platforms, and while the C standard guarantees the slightly different ((void *)0 == NULL) is true, it makes no statement concerning the memory representation of the NULL pointer.

I have not bothered addressing this non-compliance with the C standard as it would be polishing a turd. A wholesale replacement has already been proposed, and it's likely that this assumption is prevalent in the Git codebase elsewhere anyway.

-- 
David Kastrup