Web lists-archives.com

Re: [PATCH] use DIV_ROUND_UP




On 9 July 2017 at 16:01, René Scharfe <l.s.r@xxxxxx> wrote:
> Am 09.07.2017 um 15:25 schrieb Martin Ågren:
>> On 8 July 2017 at 12:35, René Scharfe <l.s.r@xxxxxx> 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.
>> ...
>>> 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).
>
> Good point; perhaps just squash this in?
>
> ---
>  sha1_name.c | 3 +--
>  1 file changed, 1 insertion(+), 2 deletions(-)
>
> diff --git a/sha1_name.c b/sha1_name.c
> index 8c513dbff6..74fcb6d788 100644
> --- a/sha1_name.c
> +++ b/sha1_name.c
> @@ -489,8 +489,7 @@ int find_unique_abbrev_r(char *hex, const unsigned char *sha1, int len)
>                  * We now know we have on the order of 2^len objects, which
>                  * expects a collision at 2^(len/2). But we also care about hex
>                  * chars, not bits, and there are 4 bits per hex. So all
> -                * together we need to divide by 2; but we also want to round
> -                * odd numbers up, hence adding one before dividing.
> +                * together we need to divide by 2 and round up.
>                  */
>                 len = DIV_ROUND_UP(len, 2);
>                 /*

Much better than my suggestions.