From 06f149334eb349d8ac4424cd6a5415f6d6f7042c Mon Sep 17 00:00:00 2001
From: PengZheng <howtofly@gmail.com>
Date: Thu, 28 Mar 2024 23:13:15 +0800
Subject: [PATCH] gh-674: Fix broken hash and compare functions of version.

---
 libs/utils/gtest/src/VersionTestSuite.cc | 22 ++++++++++++++++++++++
 libs/utils/src/version.c                 | 16 ++++++++--------
 libs/utils/src/version_private.h         |  1 +
 3 files changed, 31 insertions(+), 8 deletions(-)

diff --git a/libs/utils/gtest/src/VersionTestSuite.cc b/libs/utils/gtest/src/VersionTestSuite.cc
index 762f9c885..c3d9118c5 100644
--- a/libs/utils/gtest/src/VersionTestSuite.cc
+++ b/libs/utils/gtest/src/VersionTestSuite.cc
@@ -136,6 +136,20 @@ TEST_F(VersionTestSuite, CompareTest) {
     EXPECT_TRUE(result > 0);
     celix_version_destroy(compare);
 
+    // Compare against a lower version
+    compare = celix_version_create(1, 2, 2, str);
+    EXPECT_TRUE(compare != nullptr);
+    result = celix_version_compareTo(version, compare);
+    EXPECT_TRUE(result > 0);
+    celix_version_destroy(compare);
+
+    // Compare against a lower version
+    compare = celix_version_create(1, 2, 3, nullptr);
+    EXPECT_TRUE(compare != nullptr);
+    result = celix_version_compareTo(version, compare);
+    EXPECT_TRUE(result > 0);
+    celix_version_destroy(compare);
+
     celix_version_destroy(version);
 }
 
@@ -303,3 +317,11 @@ TEST_F(VersionTestSuite, ParseTest) {
     EXPECT_EQ(nullptr, result);
     EXPECT_STREQ("Invalid version component(2)", celix_err_popLastError());
 }
+
+TEST_F(VersionTestSuite, HashTest) {
+    celix_autoptr(celix_version_t) ver1 = celix_version_create(1, 1, 0, nullptr);
+    celix_autoptr(celix_version_t) ver2 = celix_version_create(1, 0, 1, nullptr);
+    celix_autoptr(celix_version_t) ver3 = celix_version_create(1, 1, 0, "abc");
+    EXPECT_NE(celix_version_hash(ver1), celix_version_hash(ver2));
+    EXPECT_NE(celix_version_hash(ver1), celix_version_hash(ver3));
+}
\ No newline at end of file
diff --git a/libs/utils/src/version.c b/libs/utils/src/version.c
index f1250a618..50aff1be7 100644
--- a/libs/utils/src/version.c
+++ b/libs/utils/src/version.c
@@ -185,13 +185,7 @@ int celix_version_compareTo(const celix_version_t* version, const celix_version_
                 if (res != 0) {
                     result = res;
                 } else {
-                    if(celix_utils_isStringNullOrEmpty(version->qualifier) && celix_utils_isStringNullOrEmpty(version->qualifier)) {
-                        result = 0;
-                    } else if (celix_utils_isStringNullOrEmpty(version->qualifier) || celix_utils_isStringNullOrEmpty(version->qualifier)) {
-                        result = -1;
-                    } else {
-                        result = strcmp(version->qualifier, compare->qualifier);
-                    }
+                    result = strcmp(version->qualifier, compare->qualifier);
                 }
             }
         }
@@ -244,7 +238,13 @@ bool celix_version_isUserCompatible(const celix_version_t* user, int providerMaj
 }
 
 unsigned int celix_version_hash(const celix_version_t* version) {
-    return (unsigned int)(version->major | version->minor | version->micro | celix_utils_stringHash(version->qualifier));
+    unsigned int h = 0;
+    h = 31 * 17;
+    h = 31 * h + (unsigned int)version->major;
+    h = 31 * h + (unsigned int)version->minor;
+    h = 31 * h + (unsigned int)version->micro;
+    h = 31 * h + celix_utils_stringHash(version->qualifier);
+    return h;
 }
 
 int celix_version_compareToMajorMinor(const celix_version_t* version, int majorVersionPart, int minorVersionPart) {
diff --git a/libs/utils/src/version_private.h b/libs/utils/src/version_private.h
index 78ec2daa5..cadd54142 100644
--- a/libs/utils/src/version_private.h
+++ b/libs/utils/src/version_private.h
@@ -25,6 +25,7 @@ struct celix_version {
     int minor;
     int micro;
     char *qualifier;
+    int hash;
 };