Eric Blake <[EMAIL PROTECTED]> 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 <[EMAIL PROTECTED]>
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
<[EMAIL PROTECTED]> with ideas from Ben Pfaff
- <[EMAIL PROTECTED]> and Bruno Haible <[EMAIL PROTECTED]>.
+ <[EMAIL PROTECTED]>, Bruno Haible <[EMAIL PROTECTED]> and Eric
+ Blake <[EMAIL PROTECTED]>.
* 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
