From: Jeff King <peff@peff.net>
To: Olivier Galibert <galibert@pobox.com>
Cc: Linus Torvalds <torvalds@linux-foundation.org>,
Junio C Hamano <junkio@cox.net>,
Git Mailing List <git@vger.kernel.org>
Subject: Re: Fix up ugly open-coded "alloc_nr()" user in object.c
Date: Sat, 16 Jun 2007 18:37:39 -0400 [thread overview]
Message-ID: <20070616223738.GA19076@coredump.intra.peff.net> (raw)
In-Reply-To: <20070616221506.GA78651@dspnet.fr.eu.org>
On Sun, Jun 17, 2007 at 12:15:06AM +0200, Olivier Galibert wrote:
> > + ALLOC_GROW(array->objects, array->nr, array->alloc);
>
> Unless the ALLOC_GROW semantics are weird, shouldn't that be:
> ALLOC_GROW(array->objects, array->nr+1, array->alloc);
The semantics are weird. They never seemed so to me before, since it was
replacing some "grow by 1" areas where it is natural to assume that you
need just one spot more. But the way Junio commented it and tweaked it,
it can handle arbitrary growth (which is much better), but that means we
are overly conservative about when to grow.
Junio, patch is below (call-sites using bare 'nr' need to be 'nr+1', but
I will fix those up in a separate patch since they are in next and this
is in master).
-- >8 --
fix ALLOC_GROW off-by-one
The ALLOC_GROW macro will never let us fill the array completely,
instead allocating an extra chunk if that would be the case. This is
because the 'nr' argument was originally treated as "how much we do have
now" instead of "how much do we want". The latter makes much more
sense because you can grow by more than one item.
This off-by-one never resulted in an error because it meant we were
overly conservative about when to allocate. Any callers which passed
"how we have now" need to be updated, or they will fail to allocate
enough.
Signed-off-by: Jeff King <peff@peff.net>
---
cache.h | 2 +-
1 files changed, 1 insertions(+), 1 deletions(-)
diff --git a/cache.h b/cache.h
index c914c1c..ed83d92 100644
--- a/cache.h
+++ b/cache.h
@@ -234,7 +234,7 @@ extern void verify_non_filename(const char *prefix, const char *name);
*/
#define ALLOC_GROW(x, nr, alloc) \
do { \
- if ((nr) >= alloc) { \
+ if ((nr) > alloc) { \
if (alloc_nr(alloc) < (nr)) \
alloc = (nr); \
else \
prev parent reply other threads:[~2007-06-16 22:37 UTC|newest]
Thread overview: 4+ messages / expand[flat|nested] mbox.gz Atom feed top
2007-06-16 17:30 Fix up ugly open-coded "alloc_nr()" user in object.c Linus Torvalds
2007-06-16 18:21 ` Jeff King
2007-06-16 22:15 ` Olivier Galibert
2007-06-16 22:37 ` Jeff King [this message]
Reply instructions:
You may reply publicly to this message via plain-text email
using any one of the following methods:
* Save the following mbox file, import it into your mail client,
and reply-to-all from there: mbox
Avoid top-posting and favor interleaved quoting:
https://en.wikipedia.org/wiki/Posting_style#Interleaved_style
* Reply using the --to, --cc, and --in-reply-to
switches of git-send-email(1):
git send-email \
--in-reply-to=20070616223738.GA19076@coredump.intra.peff.net \
--to=peff@peff.net \
--cc=galibert@pobox.com \
--cc=git@vger.kernel.org \
--cc=junkio@cox.net \
--cc=torvalds@linux-foundation.org \
/path/to/YOUR_REPLY
https://kernel.org/pub/software/scm/git/docs/git-send-email.html
* If your mail client supports setting the In-Reply-To header
via mailto: links, try the mailto: link
Be sure your reply has a Subject: header at the top and a blank line
before the message body.
This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox