Re: [PATCH v2 3/9] ewah/bitmap: introduce bitmap_word_alloc()

From: Christian Couder <christian.couder@gmail.com>
Date: Oct 26, 2019, 09:29 UTC
Message-ID: <CAP8UFD05uH7m7p0iR6g91MEd1QFd8vrJpKeKXsi0Brb9PKVQMg@mail.gmail.com>
In-Reply-To: <20191022174628.115505-1-jonathantanmy@google.com>

On Tue, Oct 22, 2019 at 7:46 PM Jonathan Tan <jonathantanmy@google.com> wrote:

Show 14 quoted lines

>
> > From: Jeff King <peff@peff.net>
> >
> > In a following commit we will need to allocate a variable
> > number of bitmap words, instead of always 32, so let's add
> > bitmap_word_alloc() for this purpose.
> >
> > We will also always access at least one word for each bitmap,
> > so we want to make sure that at least one is always
> > allocated.
>
> The last paragraph is not true - we still can allocate 0. (We just ensure
> that when we grow, we grow to at least 1.) I think we should just
> delete the last paragraph - the first paragraph is sufficient.

Ok, I removed the last paragraph in my current version.

> Other than that, all patches up to but not including the last one look
> good, except the ones that just add a new function because I'll have to
> review the last patch to see how they are used.

Thanks for your review, Christian.