gh-674: Fix broken hash and compare functions of version.

apache · Mar 28, 2024 · 06f1493 · 06f1493
1 parent f830c2a
commit 06f1493
Show file tree

Hide file tree

Showing 3 changed files with 31 additions and 8 deletions.
diff --git 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));
+}
diff --git 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
@@ -25,6 +25,7 @@ struct celix_version {
     int minor;
     int micro;
     char *qualifier;
+    int hash;
 };