[PATCH v2 01/27] viruuid: Rework virUUIDIsValid()

Michal Privoznik posted 27 patches 5 years, 2 months ago
[PATCH v2 01/27] viruuid: Rework virUUIDIsValid()
Posted by Michal Privoznik 5 years, 2 months ago
The only test we do when checking for UUID validity is that
whether all bytes are the same (invalid UUID) or not (valid
UUID). The algorithm we use is needlessly complicated.

Also, the checked UUID is not modified and hence the argument can
be of 'const' type.

Signed-off-by: Michal Privoznik <mprivozn@redhat.com>
---
 src/util/viruuid.c | 17 +++++++----------
 src/util/viruuid.h |  2 +-
 2 files changed, 8 insertions(+), 11 deletions(-)

diff --git a/src/util/viruuid.c b/src/util/viruuid.c
index 908b09945d..558fbb9c0d 100644
--- a/src/util/viruuid.c
+++ b/src/util/viruuid.c
@@ -170,25 +170,22 @@ virUUIDFormat(const unsigned char *uuid, char *uuidstr)
  * Basic tests:
  *  - Not all of the digits may be equal
  */
-int
-virUUIDIsValid(unsigned char *uuid)
+bool
+virUUIDIsValid(const unsigned char *uuid)
 {
     size_t i;
-    unsigned int ctr = 1;
-    unsigned char c;
 
     if (!uuid)
-        return 0;
-
-    c = uuid[0];
+        return false;
 
     for (i = 1; i < VIR_UUID_BUFLEN; i++)
-        if (uuid[i] == c)
-            ctr++;
+        if (uuid[i] != uuid[0])
+            return true;
 
-    return ctr != VIR_UUID_BUFLEN;
+    return false;
 }
 
+
 static int
 getDMISystemUUID(char *uuid, int len)
 {
diff --git a/src/util/viruuid.h b/src/util/viruuid.h
index 5d64e58405..b403b1906a 100644
--- a/src/util/viruuid.h
+++ b/src/util/viruuid.h
@@ -43,7 +43,7 @@
 int virSetHostUUIDStr(const char *host_uuid);
 int virGetHostUUID(unsigned char *host_uuid) ATTRIBUTE_NONNULL(1) G_GNUC_NO_INLINE;
 
-int virUUIDIsValid(unsigned char *uuid);
+bool virUUIDIsValid(const unsigned char *uuid);
 
 int virUUIDGenerate(unsigned char *uuid) G_GNUC_NO_INLINE;
 
-- 
2.26.2

Re: [PATCH v2 01/27] viruuid: Rework virUUIDIsValid()
Posted by Peter Krempa 5 years, 2 months ago
On Thu, Dec 03, 2020 at 13:36:04 +0100, Michal Privoznik wrote:
> The only test we do when checking for UUID validity is that
> whether all bytes are the same (invalid UUID) or not (valid
> UUID). The algorithm we use is needlessly complicated.
> 
> Also, the checked UUID is not modified and hence the argument can
> be of 'const' type.
> 
> Signed-off-by: Michal Privoznik <mprivozn@redhat.com>
> ---
>  src/util/viruuid.c | 17 +++++++----------
>  src/util/viruuid.h |  2 +-
>  2 files changed, 8 insertions(+), 11 deletions(-)

I've briefly thought whether we could replace the internals by a memset
of a temp array and then memcmp, but that doesn't seem to be worth.

Reviewed-by: Peter Krempa <pkrempa@redhat.com>