From: "René Scharfe" <l.s.r@web.de>
To: Stefan Beller <sbeller@google.com>
Cc: Git Mailing List <git@vger.kernel.org>,
Stefan Beller <stefanbeller@gmail.com>, Jeff King <peff@peff.net>,
Vicent Marti <tanoku@gmail.com>
Subject: Re: [PATCH] pack-bitmaps: plug memory leak, fix allocation size for recent_bitmaps
Date: Tue, 19 May 2015 22:29:18 +0200 [thread overview]
Message-ID: <555B9D1E.2050609@web.de> (raw)
In-Reply-To: <CAGZ79kZeX_VqcvTt_kgGKyWH0SV2zsV_j1KuxNhwR18zWfp5LA@mail.gmail.com>
Am 19.05.2015 um 02:57 schrieb Stefan Beller:
> On Mon, May 18, 2015 at 4:24 PM, René Scharfe <l.s.r@web.de> wrote:
>> diff --git a/pack-bitmap.c b/pack-bitmap.c
>> index 62a98cc..e5abb8a 100644
>> --- a/pack-bitmap.c
>> +++ b/pack-bitmap.c
>> @@ -209,14 +209,12 @@ static inline uint8_t read_u8(const unsigned char *buffer, size_t *pos)
>> return buffer[(*pos)++];
>> }
>>
>> +#define MAX_XOR_OFFSET 160
>> +
>> static int load_bitmap_entries_v1(struct bitmap_index *index)
>> {
>> - static const size_t MAX_XOR_OFFSET = 160;
>
> Is there a reason why we prefer defines over a static const size_t here?
Yes, see below.
>> -
>> uint32_t i;
>> - struct stored_bitmap **recent_bitmaps;
>> -
>> - recent_bitmaps = xcalloc(MAX_XOR_OFFSET, sizeof(struct stored_bitmap));
>> + struct stored_bitmap *recent_bitmaps[MAX_XOR_OFFSET] = { NULL };
If MAX_XOR_OFFSET is a const then C89 does not allow this declaration.
C99 gives you a variable-length array here. VLAs can't be initialized
at declaration time, so we'd need to add a memset() call (at least with
GCC 4.9). Overall it's simpler and shorter to use a macro.
We could also use an enum, but that's not exactly common.
René
next prev parent reply other threads:[~2015-05-19 20:29 UTC|newest]
Thread overview: 6+ messages / expand[flat|nested] mbox.gz Atom feed top
2015-05-18 23:24 [PATCH] pack-bitmaps: plug memory leak, fix allocation size for recent_bitmaps René Scharfe
2015-05-19 0:57 ` Stefan Beller
2015-05-19 20:29 ` René Scharfe [this message]
2015-05-19 2:23 ` Jeff King
2015-05-19 19:45 ` Junio C Hamano
2015-05-19 21:19 ` 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=555B9D1E.2050609@web.de \
--to=l.s.r@web.de \
--cc=git@vger.kernel.org \
--cc=peff@peff.net \
--cc=sbeller@google.com \
--cc=stefanbeller@gmail.com \
--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.