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

From: Jeff King <peff@peff.net>
Date: Dec 9, 2019, 06:28 UTC
Message-ID: <20191209062841.GC40570@coredump.intra.peff.net>
In-Reply-To: <20191115141541.11149-4-chriscool@tuxfamily.org>

On Fri, Nov 15, 2019 at 03:15:35PM +0100, Christian Couder wrote:

> 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.

This name is much better than the horrible "bitmap_new2()" it was called in the original. ;)

I wonder if "new_with_world_alloc" or "new_with_size" would make it more obvious that this is also a constructor, though.

Show 8 quoted lines

>  void bitmap_set(struct bitmap *self, size_t pos)
>  {
>  	size_t block = EWAH_BLOCK(pos);
>  
>  	if (block >= self->word_alloc) {
>  		size_t old_size = self->word_alloc;
> -		self->word_alloc = block * 2;
> +		self->word_alloc = block ? block * 2 : 1;

Since this hunk caused so much confusion, maybe worth calling it out in the commit message. Something like:

  Note that we have to adjust the block growth in bitmap_set(), since
  a caller could now use an initial size of "0" (we don't plan to do
  that, but it doesn't hurt to be defensive).

-Peff