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
