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.
