Correctly compare version numbers like '1.2.3' and '1.2.3a'

Only switch to the string comparison after the numeric part has been used.

Fixes https://github.com/hughsie/appstream-glib/issues/231

2 files changed, 28 insertions, 33 deletions

diff --git a/libappstream-glib/as-self-test.c b/libappstream-glib/as-self-test.c
index 8f4e6e9..ead8c81 100644
--- a/libappstream-glib/as-self-test.c
+++ b/libappstream-glib/as-self-test.c
@@ -5032,9 +5032,18 @@ as_test_utils_vercmp_func (void)
 	g_assert_cmpint (as_utils_vercmp ("1.2.3.1", "1.2.4"), <, 0);
 
 	/* mixed-alpha-numeric */
-	g_assert_cmpint (as_utils_vercmp ("1.2xxx.3", "1.2.3"), ==, G_MAXINT);
-	g_assert_cmpint (as_utils_vercmp ("1.2.3", "1.2xxx.3"), ==, G_MAXINT);
-	g_assert_cmpint (as_utils_vercmp ("1.2.-3", "1.2.3"), ==, G_MAXINT);
+	g_assert_cmpint (as_utils_vercmp ("1.2.3a", "1.2.3a"), ==, 0);
+	g_assert_cmpint (as_utils_vercmp ("1.2.3a", "1.2.3b"), <, 0);
+	g_assert_cmpint (as_utils_vercmp ("1.2.3b", "1.2.3a"), >, 0);
+
+	/* alpha version append */
+	g_assert_cmpint (as_utils_vercmp ("1.2.3", "1.2.3a"), <, 0);
+	g_assert_cmpint (as_utils_vercmp ("1.2.3a", "1.2.3"), >, 0);
+
+	/* alpha only */
+	g_assert_cmpint (as_utils_vercmp ("alpha", "alpha"), ==, 0);
+	g_assert_cmpint (as_utils_vercmp ("alpha", "beta"), <, 0);
+	g_assert_cmpint (as_utils_vercmp ("beta", "alpha"), >, 0);
 
 	/* alpha-compare */
 	g_assert_cmpint (as_utils_vercmp ("1.2a.3", "1.2a.3"), ==, 0);
diff --git a/libappstream-glib/as-utils.c b/libappstream-glib/as-utils.c
index eab3242..c605cd1 100644
--- a/libappstream-glib/as-utils.c
+++ b/libappstream-glib/as-utils.c
@@ -1368,10 +1368,6 @@ as_utils_search_tokenize (const gchar *search)
 gint
 as_utils_vercmp (const gchar *version_a, const gchar *version_b)
 {
-	gchar *endptr;
-	gint64 ver_a;
-	gint64 ver_b;
-	guint i;
 	guint longest_split;
 	g_autofree gchar *str_a = NULL;
 	g_autofree gchar *str_b = NULL;
@@ -1392,9 +1388,11 @@ as_utils_vercmp (const gchar *version_a, const gchar *version_b)
 	split_a = g_strsplit (str_a, ".", -1);
 	split_b = g_strsplit (str_b, ".", -1);
 	longest_split = MAX (g_strv_length (split_a), g_strv_length (split_b));
-	for (i = 0; i < longest_split; i++) {
-		gboolean isnum_a = TRUE;
-		gboolean isnum_b = TRUE;
+	for (guint i = 0; i < longest_split; i++) {
+		gchar *endptr_a = NULL;
+		gchar *endptr_b = NULL;
+		gint64 ver_a;
+		gint64 ver_b;
 
 		/* we lost or gained a dot */
 		if (split_a[i] == NULL)
@@ -1403,32 +1401,20 @@ as_utils_vercmp (const gchar *version_a, const gchar *version_b)
 			return 1;
 
 		/* compare integers */
-		ver_a = g_ascii_strtoll (split_a[i], &endptr, 10);
-		if (endptr != NULL && endptr[0] != '\0')
-			isnum_a = FALSE;
-		if (ver_a < 0)
-			isnum_a = FALSE;
-		ver_b = g_ascii_strtoll (split_b[i], &endptr, 10);
-		if (endptr != NULL && endptr[0] != '\0')
-			isnum_b = FALSE;
-		if (ver_b < 0)
-			isnum_b = FALSE;
-
-		/* can't compare integer with string */
-		if (isnum_a != isnum_b)
-			return G_MAXINT;
+		ver_a = g_ascii_strtoll (split_a[i], &endptr_a, 10);
+		ver_b = g_ascii_strtoll (split_b[i], &endptr_b, 10);
+		if (ver_a < ver_b)
+			return -1;
+		if (ver_a > ver_b)
+			return 1;
 
 		/* compare strings */
-		if (!isnum_a) {
-			gint rc = g_strcmp0 (split_a[i], split_b[i]);
-			if (rc != 0)
-				return rc;
-
-		/* compare integers */
-		} else {
-			if (ver_a < ver_b)
+		if ((endptr_a != NULL && endptr_a[0] != '\0') ||
+		    (endptr_b != NULL && endptr_b[0] != '\0')) {
+			gint rc = g_strcmp0 (endptr_a, endptr_b);
+			if (rc < 0)
 				return -1;
-			if (ver_a > ver_b)
+			if (rc > 0)
 				return 1;
 		}
 	}