Re: bitrotate

[Simon Josefsson]

Eric Blake <address@hidden> writes:

> Simon Josefsson <simon <at> josefsson.org> writes:
>
>> 
>> Thanks for ideas.  I have pushed this.
>> 
>> 
>> +#if defined UINT64_MAX && defined UINT64_C
>
> Bruno's point was that UINT64_C is not usable in C++ programs.  Checking 
> UINT64_MAX is sufficient, so lose the &&.
>
>> +/* Given an unsigned 64-bit argument X, return the value corresponding
>> +   to rotating the bits N steps to the left.  N must be between 1 and
>> +   31 inclusive. */
>
> s/31/63/ (two instances in your patch).
>
>> +
>> +#if defined(UINT64_MAX) && defined(UINT64_C)
>
> The preferred gnulib style avoids excess parens:
>
> #if defined UINT64_MAX && defined UINT64_C
>
> although, as mentioned for C++, this is better written as:
>
> #ifdef UINT64_MAX
>
>> +#endif
>
> Personally, I like to see a comment of what an #endif matches, when it is 
> more 
> than a screenful away.

Thank you for careful review!  I have pushed this.

/Simon

>From b729ee98921bac8c8687a4d1a941aa0567f8196e Mon Sep 17 00:00:00 2001
From: Simon Josefsson <address@hidden>
Date: Tue, 2 Sep 2008 16:59:57 +0200
Subject: [PATCH] bitrotate: Code improvements.

---
 ChangeLog              |    3 ++-
 lib/bitrotate.h        |    6 +++---
 tests/test-bitrotate.c |    4 ++--
 3 files changed, 7 insertions(+), 6 deletions(-)

diff --git a/ChangeLog b/ChangeLog
index 58c6e5b..b69ee9c 100644
--- a/ChangeLog
+++ b/ChangeLog
@@ -2,7 +2,8 @@
 
        * lib/bitrotate.h (rotl64, rotr64): Add.  Suggested by Bruce Korb
        <address@hidden> with ideas from Ben Pfaff
-       <address@hidden> and Bruno Haible <address@hidden>.
+       <address@hidden>, Bruno Haible <address@hidden> and Eric
+       Blake <address@hidden>.
 
        * tests/test-bitrotate.c: Add more test vectors.
 
diff --git a/lib/bitrotate.h b/lib/bitrotate.h
index b8528b4..65bf682 100644
--- a/lib/bitrotate.h
+++ b/lib/bitrotate.h
@@ -21,10 +21,10 @@
 
 #include <stdint.h>
 
-#if defined UINT64_MAX && defined UINT64_C
+#ifdef UINT64_MAX
 /* Given an unsigned 64-bit argument X, return the value corresponding
    to rotating the bits N steps to the left.  N must be between 1 and
-   31 inclusive. */
+   63 inclusive. */
 static inline uint64_t
 rotl64 (uint64_t x, int n)
 {
@@ -33,7 +33,7 @@ rotl64 (uint64_t x, int n)
 
 /* Given an unsigned 64-bit argument X, return the value corresponding
    to rotating the bits N steps to the right.  N must be between 1 to
-   31 inclusive.*/
+   63 inclusive.*/
 static inline uint64_t
 rotr64 (uint64_t x, int n)
 {
diff --git a/tests/test-bitrotate.c b/tests/test-bitrotate.c
index 41f4d26..5b7d171 100644
--- a/tests/test-bitrotate.c
+++ b/tests/test-bitrotate.c
@@ -151,7 +151,7 @@ main (void)
   ASSERT (rotr32 (2309737967U, 30) == 649017278lU);
   ASSERT (rotr32 (2309737967U, 31) == 324508639lU);
 
-#if defined(UINT64_MAX) && defined(UINT64_C)
+#ifdef UINT64_MAX
   ASSERT (rotl64 (16045690984503098046ULL, 1) == 13644637895296644477ULL);
   ASSERT (rotl64 (16045690984503098046ULL, 2) == 8842531716883737339ULL);
   ASSERT (rotl64 (16045690984503098046ULL, 3) == 17685063433767474678ULL);
@@ -280,7 +280,7 @@ main (void)
   ASSERT (rotr64 (16045690984503098046ULL, 61) == 17685063433767474678ULL);
   ASSERT (rotr64 (16045690984503098046ULL, 62) == 8842531716883737339ULL);
   ASSERT (rotr64 (16045690984503098046ULL, 63) == 13644637895296644477ULL);
-#endif
+#endif /* UINT64_MAX */
 
   return 0;
 }
-- 
1.5.6.3