Re: [PATCH] use DIV_ROUND_UP

From: Martin Ågren <martin.agren@gmail.com>
Date: Jul 9, 2017, 13:25 UTC
Message-ID: <CAN0heSr0qJv7VqaasLGNFbNDbqYOoqmi3FyLkb=co4E10Y9VOg@mail.gmail.com>
In-Reply-To: <4d2c274a-f6cb-6ea5-304f-51a3152cc436@web.de>

On 8 July 2017 at 12:35, René Scharfe <l.s.r@web.de> 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.

...

Show 10 quoted lines

> diff --git a/sha1_name.c b/sha1_name.c
> index e7f7b12ceb..8c513dbff6 100644
> --- a/sha1_name.c
> +++ b/sha1_name.c
> @@ -492,7 +492,7 @@ int find_unique_abbrev_r(char *hex, const unsigned char *sha1, int len)
>                  * together we need to divide by 2; but we also want to round
>                  * odd numbers up, hence adding one before dividing.
>                  */
> -               len = (len + 1) / 2;
> +               len = DIV_ROUND_UP(len, 2);

Since the addition is now an implementation detail of DIV_ROUND_UP, should the comment be adjusted, maybe simply by removing ", hence adding one before dividing"?

Or perhaps even better, "... divide by 2; but since len might be odd, we need to make sure we round up as we divide". My thinking being, we're not actually rounding odd numbers up (presumably to even numbers), but we're rounding the result of the division up (to the smallest larger integer).