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

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

[David Kastrup]

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));
 			sb->lineno[num] = buf - sb->final_buf;
 			bol = 0;
 		}
@@ -1789,7 +1789,7 @@ static int prepare_lines(struct scoreboard *sb)
 		}
 	}
 	sb->lineno = xrealloc(sb->lineno,
-			      sizeof(int *) * (num + incomplete + 1));
+			      sizeof(int) * (num + incomplete + 1));
 	sb->lineno[num + incomplete] = buf - sb->final_buf;
 	sb->num_lines = num + incomplete;
 	return sb->num_lines;
-- 
1.8.3.2

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

[David Kastrup]

David Kastrup <dak@gnu.org> writes:

> 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

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

[Jeff King]

On Sat, Feb 08, 2014 at 10:49:40AM +0100, David Kastrup wrote:

> 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.

Yes, we explicitly break this part of the standard in the name of
practicality (it simplifies frequently-used code, and machines on which
it matters are rare enough that nobody has ever complained about it).

So I do not think this is a problem.

However, is there a reason not to use:

  sizeof(*sb->lineno)

rather than

  sizeof(int)

to avoid type-mismatch errors entirely (this applies both to this patch,
and to any proposed rewrites using malloc).

-Peff

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

[David Kastrup]

Jeff King <peff@peff.net> writes:

> However, is there a reason not to use:
>
>   sizeof(*sb->lineno)
>
> rather than
>
>   sizeof(int)
>
> to avoid type-mismatch errors entirely (this applies both to this patch,
> and to any proposed rewrites using malloc).

It deviates from the style of the original code by tried and true Git
developers.  So feel free to roll your own patch here: it's not like
this one has any copyrightable content in it.

-- 
David Kastrup