Libguestfs - May 2023 - [PATCH nbdkit v2 4/6] common/include: Make log_2_bits work on 64 bit ints

Previously it only worked for 64 bit ints on 64 bit platforms, but we
can easily adjust the function to work on any platform.
---
 common/include/ispowerof2.h      | 4 ++--
 common/include/test-ispowerof2.c | 6 ++----
 2 files changed, 4 insertions(+), 6 deletions(-)

diff --git a/common/include/ispowerof2.h b/common/include/ispowerof2.h
index a4cb52de3..8f56c8dbf 100644
--- a/common/include/ispowerof2.h
+++ b/common/include/ispowerof2.h
@@ -54,9 +54,9 @@ is_power_of_2 (unsigned long v)
  * __builtin_clzl is available in GCC and clang.
  */
 static inline int
-log_2_bits (unsigned long v)
+log_2_bits (uint64_t v)
 {
-  return SIZEOF_LONG*8 - __builtin_clzl (v) - 1;
+  return 64 - __builtin_clzll (v) - 1;
 }
 
 /* Round up to next power of 2.
diff --git a/common/include/test-ispowerof2.c b/common/include/test-ispowerof2.c
index 1221ac09c..09d248889 100644
--- a/common/include/test-ispowerof2.c
+++ b/common/include/test-ispowerof2.c
@@ -63,10 +63,8 @@ main (void)
   assert (log_2_bits (512) == 9);
   assert (log_2_bits (4096) == 12);
   assert (log_2_bits (0x80000000) == 31);
-#if SIZEOF_LONG == 8
-  assert (log_2_bits (0x100000000) == 32);
-  assert (log_2_bits (0x8000000000000000) == 63);
-#endif
+  assert (log_2_bits (UINT64_C (0x100000000)) == 32);
+  assert (log_2_bits (UINT64_C (0x8000000000000000)) == 63);
 
   /* Test next power of 2. */
   assert (next_power_of_2 (0) == 1);
-- 
2.39.2

On Wed, May 17, 2023 at 11:06:57AM +0100, Richard W.M. Jones
wrote:
> Previously it only worked for 64 bit ints on 64 bit platforms, but we
> can easily adjust the function to work on any platform.
> ---
>  common/include/ispowerof2.h      | 4 ++--
>  common/include/test-ispowerof2.c | 6 ++----
>  2 files changed, 4 insertions(+), 6 deletions(-)
> 
> diff --git a/common/include/ispowerof2.h b/common/include/ispowerof2.h
> index a4cb52de3..8f56c8dbf 100644
> --- a/common/include/ispowerof2.h
> +++ b/common/include/ispowerof2.h
> @@ -54,9 +54,9 @@ is_power_of_2 (unsigned long v)
>   * __builtin_clzl is available in GCC and clang.
>   */
>  static inline int
> -log_2_bits (unsigned long v)
> +log_2_bits (uint64_t v)
>  {
> -  return SIZEOF_LONG*8 - __builtin_clzl (v) - 1;
> +  return 64 - __builtin_clzll (v) - 1;
We already documented that this is undefined for v==0, and your change
does not alter that.

What we don't document is whether this is well-defined for a
non-power-of-2...
>  }
>  
>  /* Round up to next power of 2.
> diff --git a/common/include/test-ispowerof2.c
b/common/include/test-ispowerof2.c
> index 1221ac09c..09d248889 100644
> --- a/common/include/test-ispowerof2.c
> +++ b/common/include/test-ispowerof2.c
> @@ -63,10 +63,8 @@ main (void)
>    assert (log_2_bits (512) == 9);
>    assert (log_2_bits (4096) == 12);
>    assert (log_2_bits (0x80000000) == 31);
> -#if SIZEOF_LONG == 8
> -  assert (log_2_bits (0x100000000) == 32);
> -  assert (log_2_bits (0x8000000000000000) == 63);
> -#endif
> +  assert (log_2_bits (UINT64_C (0x100000000)) == 32);
> +  assert (log_2_bits (UINT64_C (0x8000000000000000)) == 63);
...we only test it on values where is_power_of_2() returns true.  We
should probably take this opportunity to either document that it is
not just 0, but all non-power-of-2 values that have undefined
behavior; or else to test some values with more than one bit set.

-- 
Eric Blake, Principal Software Engineer
Red Hat, Inc.           +1-919-301-3266
Virtualization:  qemu.org | libvirt.org

On Wed, May 17, 2023 at 11:06:57AM +0100, Richard W.M. Jones
wrote:
> Previously it only worked for 64 bit ints on 64 bit platforms, but we
> can easily adjust the function to work on any platform.
> ---
>  common/include/ispowerof2.h      | 4 ++--
>  common/include/test-ispowerof2.c | 6 ++----
>  2 files changed, 4 insertions(+), 6 deletions(-)
Should we also fix is_power_of_2() to work on 64-bit values on all
platforms?

-- 
Eric Blake, Principal Software Engineer
Red Hat, Inc.           +1-919-301-3266
Virtualization:  qemu.org | libvirt.org