Re: [PATCH] builtin/blame.c::prepare_lines:...

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

From: David Kastrup <hidden>
Date: 2016-06-15 22:59:52

David Kastrup [off-list ref] writes:

quoted hunk

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 <redacted>
---
This should be less contentious than the patch in
<URL:http://permalink.gmane.org/gmane.comp.version-control.git/241561>,
Message-ID: [off-list ref] 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

From: Jeff King <hidden>
Date: 2016-06-15 22:59:52

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

`h`	back out one level
`j`	next message in thread
`k`	previous message in thread
`l`	drill in
`Esc`	close help / fold thread tree
`?`	toggle this help