Show 46 quoted lines
> Am 09.07.2017 um 15:25 schrieb Martin Ågren:
>> 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.
>> ...
>>> 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);
> /*