All of lore.kernel.org
 help / color / mirror / Atom feed
From: David Kastrup <dak@gnu.org>
To: Jeff King <peff@peff.net>
Cc: "René Scharfe" <l.s.r@web.de>,
	"Git Mailing List" <git@vger.kernel.org>,
	"Junio C Hamano" <gitster@pobox.com>,
	"Vicent Marti" <tanoku@gmail.com>
Subject: Re: [PATCH] pack-objects: use free()+xcalloc() instead of xrealloc()+memset()
Date: Mon, 02 Jun 2014 22:40:44 +0200	[thread overview]
Message-ID: <878upf9h9v.fsf@fencepost.gnu.org> (raw)
In-Reply-To: <20140602194246.GD2510@sigill.intra.peff.net> (Jeff King's message of "Mon, 2 Jun 2014 15:42:46 -0400")

Jeff King <peff@peff.net> writes:

> On Sun, Jun 01, 2014 at 01:07:21PM +0200, René Scharfe wrote:
>
>> Whenever the hash table becomes too small then its size is increased,
>> the original part (and the added space) is zerod out using memset(),
>> and the table is rebuilt from scratch.
>> 
>> Simplify this proceess by returning the old memory using free() and
>> allocating the new buffer using xcalloc(), which already clears the
>> buffer for us.  That way we avoid copying the old hash table contents
>> needlessly inside xrealloc().
>> 
>> While at it, use the first array member with sizeof instead of a
>> specific type.  The old code used uint32_t and int, while index is
>> actually an array of int32_t.  Their sizes are the same basically
>> everywhere, so it's not actually a problem, but the new code is
>> cleaner and doesn't have to be touched should the type be changed.
>> 
>> Signed-off-by: Rene Scharfe <l.s.r@web.de>
>
> Looks good to me.
>
> BTW, the code does git-blame to Vicent's 2834bc2 (which I also worked
> on), but actually originated in 7a979d9 (Thin pack - create packfile
> with missing delta base., 2006-02-19). Not that it matters, but I was
> just surprised since the code you are changing did not seem familiar to
> me. I guess there was just too much refactoring during the code movement
> for git-blame to pass along the blame in this case.

Without -M, "too much refactoring" for git-blame may just be moving a
function to a different place in the same file.

-- 
David Kastrup

  reply	other threads:[~2014-06-02 20:41 UTC|newest]

Thread overview: 6+ messages / expand[flat|nested]  mbox.gz  Atom feed  top
2014-06-01 11:07 [PATCH] pack-objects: use free()+xcalloc() instead of xrealloc()+memset() René Scharfe
2014-06-02 19:42 ` Jeff King
2014-06-02 20:40   ` David Kastrup [this message]
2014-06-02 21:59     ` Jeff King
2014-06-02 23:09       ` Junio C Hamano
2014-06-03  6:29         ` Jeff King

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=878upf9h9v.fsf@fencepost.gnu.org \
    --to=dak@gnu.org \
    --cc=git@vger.kernel.org \
    --cc=gitster@pobox.com \
    --cc=l.s.r@web.de \
    --cc=peff@peff.net \
    --cc=tanoku@gmail.com \
    /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 an external index of several public inboxes,
see mirroring instructions on how to clone and mirror
all data and code used by this external index.