[PATCH v2 02/19] host-utils: move abs64() to host-utils as uabs64()

Luis Pires posted 19 patches 4 years, 5 months ago
Maintainers: "Michael S. Tsirkin" <mst@redhat.com>, Eduardo Habkost <ehabkost@redhat.com>, Richard Henderson <richard.henderson@linaro.org>, Paolo Bonzini <pbonzini@redhat.com>, Greg Kurz <groug@kaod.org>, Damien Hedde <damien.hedde@greensocs.com>, Marcel Apfelbaum <marcel.apfelbaum@gmail.com>, Luc Michel <luc@lmichel.fr>, David Gibson <david@gibson.dropbear.id.au>
There is a newer version of this series
[PATCH v2 02/19] host-utils: move abs64() to host-utils as uabs64()
Posted by Luis Pires 4 years, 5 months ago
Move abs64 to host-utils as uabs64, so it can be used elsewhere.
The function was renamed to uabs64 and modified to return an
unsigned value. This avoids the undefined behavior for common
abs implementations, where abs of the most negative value is
undefined.

Signed-off-by: Luis Pires <luis.pires@eldorado.org.br>
---
 hw/i386/kvm/i8254.c       | 7 +------
 include/qemu/host-utils.h | 8 ++++++++
 2 files changed, 9 insertions(+), 6 deletions(-)

diff --git a/hw/i386/kvm/i8254.c b/hw/i386/kvm/i8254.c
index fa68669e8a..191a26fa57 100644
--- a/hw/i386/kvm/i8254.c
+++ b/hw/i386/kvm/i8254.c
@@ -59,11 +59,6 @@ struct KVMPITClass {
     DeviceRealize parent_realize;
 };
 
-static int64_t abs64(int64_t v)
-{
-    return v < 0 ? -v : v;
-}
-
 static void kvm_pit_update_clock_offset(KVMPITState *s)
 {
     int64_t offset, clock_offset;
@@ -81,7 +76,7 @@ static void kvm_pit_update_clock_offset(KVMPITState *s)
         clock_gettime(CLOCK_MONOTONIC, &ts);
         offset -= ts.tv_nsec;
         offset -= (int64_t)ts.tv_sec * 1000000000;
-        if (abs64(offset) < abs64(clock_offset)) {
+        if (uabs64(offset) < uabs64(clock_offset)) {
             clock_offset = offset;
         }
     }
diff --git a/include/qemu/host-utils.h b/include/qemu/host-utils.h
index 711b221704..0c6715774c 100644
--- a/include/qemu/host-utils.h
+++ b/include/qemu/host-utils.h
@@ -357,6 +357,14 @@ static inline uint64_t revbit64(uint64_t x)
 #endif
 }
 
+/**
+ * Return the absolute value of a 64-bit integer as an unsigned 64-bit value
+ */
+static inline uint64_t uabs64(int64_t v)
+{
+    return v < 0 ? -v : v;
+}
+
 /**
  * sadd32_overflow - addition with overflow indication
  * @x, @y: addends
-- 
2.25.1


Re: [PATCH v2 02/19] host-utils: move abs64() to host-utils as uabs64()
Posted by Richard Henderson 4 years, 5 months ago
On 8/31/21 9:39 AM, Luis Pires wrote:
> Move abs64 to host-utils as uabs64, so it can be used elsewhere.
> The function was renamed to uabs64 and modified to return an
> unsigned value. This avoids the undefined behavior for common
> abs implementations, where abs of the most negative value is
> undefined.
> 
> Signed-off-by: Luis Pires<luis.pires@eldorado.org.br>
> ---
>   hw/i386/kvm/i8254.c       | 7 +------
>   include/qemu/host-utils.h | 8 ++++++++
>   2 files changed, 9 insertions(+), 6 deletions(-)

Reviewed-by: Richard Henderson <richard.henderson@linaro.org>

r~

Re: [PATCH v2 02/19] host-utils: move abs64() to host-utils as uabs64()
Posted by Eduardo Habkost 4 years, 5 months ago
On Tue, Aug 31, 2021 at 01:39:50PM -0300, Luis Pires wrote:
> Move abs64 to host-utils as uabs64, so it can be used elsewhere.
> The function was renamed to uabs64 and modified to return an
> unsigned value. This avoids the undefined behavior for common
> abs implementations, where abs of the most negative value is
> undefined.
> 
> Signed-off-by: Luis Pires <luis.pires@eldorado.org.br>

Reviewed-by: Eduardo Habkost <ehabkost@redhat.com>

In case you need to send a new version of the series, I'd suggest
separating this into two patches: creation of the uabs64()
function in host-utils, and then changing the KVM PIT code to use
the new uabs64() function.  This way, the rest of your series
won't depend on the KVM PIT changes, helping bisectability and
backportability.

-- 
Eduardo