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

2014-02-08 Thread David Kastrup
David Kastrup d...@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 d...@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-...@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
--
To unsubscribe from this list: send the line unsubscribe git in
the body of a message to majord...@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html


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

2014-02-08 Thread 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
--
To unsubscribe from this list: send the line unsubscribe git in
the body of a message to majord...@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html


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

2014-02-08 Thread David Kastrup
Jeff King p...@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
--
To unsubscribe from this list: send the line unsubscribe git in
the body of a message to majord...@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html