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); /* -- 2.13.2
