Re: [PATCH] use DIV_ROUND_UP

[Jeff King <peff@peff.net>]

On Sat, Jul 08, 2017 at 12:35:35PM +0200, René Scharfe wrote:

> Convert code that divides and rounds up to use DIV_ROUND_UP to make the
> intent clearer and reduce the number of magic constants.

Sounds like a good idea.

> -	auto_threshold = (gc_auto_threshold + 255) / 256;
> +	auto_threshold = DIV_ROUND_UP(gc_auto_threshold, 256);

DIV_ROUND_UP(n,d) is defined as (n+d-1)/d. So this is clearly a
mechanical conversion and thus correct. And most cases are like this,
but...

> diff --git a/ewah/ewah_bitmap.c b/ewah/ewah_bitmap.c
> index 2dc9c82ecf..06c479f70a 100644
> --- a/ewah/ewah_bitmap.c
> +++ b/ewah/ewah_bitmap.c
> @@ -210,8 +210,8 @@ size_t ewah_add(struct ewah_bitmap *self, eword_t word)
>  void ewah_set(struct ewah_bitmap *self, size_t i)
>  {
>  	const size_t dist =
> -		(i + BITS_IN_EWORD) / BITS_IN_EWORD -
> -		(self->bit_size + BITS_IN_EWORD - 1) / BITS_IN_EWORD;
> +		DIV_ROUND_UP(i + 1, BITS_IN_EWORD) -
> +		DIV_ROUND_UP(self->bit_size, BITS_IN_EWORD);

...this first one is a bit trickier. Our "n" in the first one is now
"i+1".  But that's because the original was implicitly canceling the
"-1" and "+1" terms.

So I think it's a true mechanical conversion, but I have to admit the
original is confusing. Without digging I suspect it's correct, though,
just because a simple bug here would mean that our ewah bitmaps totally
don't work. So it's probably not worth spending time on.

> [...]

And all the others are straight-forward and obviously correct. So this
patch looks good to me.

-Peff