Re: [PATCH] builtin/blame.c::prepare_lines: fix allocation size of sb-lineno
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
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
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