Use a MS-style version number when parsing a hex version

If we're comparing versions that are both converted to strings it doesn't
really matter how the strings are stored (e.g. AA.BB.CC.DD or AA.BB.CCDD) as
the integer rules still apply **if** we don't remove the prefixed zeros.

The only time we have to be careful is where the firmware metainfo uses the
triplet and the .inf file uses the hex version format.

By using the triplet-style by default we match what Intel and Microsoft are
providing in the firmware release notes.

Basically:

 * If you're not using the MS-style versioning scheme, just use hex numbers in
   both places.

 * If you don't care about applying the update on Microsoft Windows, just
   don't supply an .inf file at all.

Fixes the other half of https://github.com/hughsie/fwupd/issues/34

This does break API, but we've never had the method in a released version so
I think it's fine to break at this stage. Make sure you update fwupd at the same
time if you're using git master.

The last bit of this fix would to be somehow encode the "display version" onto
the AppStream <release> object, but this needs discussion upstream first.

7 files changed, 56 insertions, 41 deletions

diff --git a/data/tests/firmware/2_0_0/firmware.inf b/data/tests/firmware/2_0_0/firmware.inf
index af863bf..bf64f8b 100644
--- a/data/tests/firmware/2_0_0/firmware.inf
+++ b/data/tests/firmware/2_0_0/firmware.inf
@@ -12,7 +12,7 @@ firmware.bin
 
 [Firmware_AddReg]
 HKR,,FirmwareId,,{84f40464-9272-4ef7-9399-cd95f12da696}
-HKR,,FirmwareVersion,%REG_DWORD%,0x20000
+HKR,,FirmwareVersion,%REG_DWORD%,0x2000000
 HKR,,FirmwareFilename,,firmware.bin
 
 [Strings]
diff --git a/data/tests/firmware/2_0_1/firmware.inf b/data/tests/firmware/2_0_1/firmware.inf
index 41664ae..83b181d 100644
--- a/data/tests/firmware/2_0_1/firmware.inf
+++ b/data/tests/firmware/2_0_1/firmware.inf
@@ -11,7 +11,7 @@ firmware.bin
 
 [Firmware_AddReg]
 HKR,,FirmwareId,,{84f40464-9272-4ef7-9399-cd95f12da696}
-HKR,,FirmwareVersion,%REG_DWORD%,0x20001
+HKR,,FirmwareVersion,%REG_DWORD%,0x2000001
 HKR,,FirmwareFilename,,firmware.bin
 
 [Strings]
diff --git a/data/tests/firmware/2_0_2/firmware.inf b/data/tests/firmware/2_0_2/firmware.inf
index 1261c0a..e5807ce 100644
--- a/data/tests/firmware/2_0_2/firmware.inf
+++ b/data/tests/firmware/2_0_2/firmware.inf
@@ -11,7 +11,7 @@ firmware.bin
 
 [Firmware_AddReg]
 HKR,,FirmwareId,,{84f40464-9272-4ef7-9399-cd95f12da696}
-HKR,,FirmwareVersion,%REG_DWORD%,0x00020002
+HKR,,FirmwareVersion,%REG_DWORD%,0x02000002
 HKR,,FirmwareFilename,,firmware.bin
 
 [Strings]
diff --git a/libappstream-builder/asb-self-test.c b/libappstream-builder/asb-self-test.c
index 2d8bc2c..9c99652 100644
--- a/libappstream-builder/asb-self-test.c
+++ b/libappstream-builder/asb-self-test.c
@@ -522,7 +522,7 @@ asb_test_context_test_func (AsbTestContextMode mode)
 		"<releases>\n"
 		"<release version=\"2.0.2\" timestamp=\"1424116753\">\n"
 		"<location>http://www.hughski.com/downloads/colorhug2/firmware/colorhug-2.0.2.cab</location>\n"
-		"<checksum filename=\"colorhug-als-2.0.2.cab\" target=\"container\" type=\"sha1\">0608b5e8169706f1351a6351630503de75939a86</checksum>\n"
+		"<checksum filename=\"colorhug-als-2.0.2.cab\" target=\"container\" type=\"sha1\">7e179f45d2782c3c9744495dd4bbd91ad3d9e841</checksum>\n"
 		"<checksum filename=\"firmware.bin\" target=\"content\" type=\"sha1\">767a8a7b8a7b350b513f57761204b4aaa657aa44</checksum>\n"
 		"<description><p>This unstable release adds the following features:</p>"
 		"<ul><li>Add TakeReadingArray to enable panel latency measurements</li>"
@@ -836,7 +836,7 @@ asb_test_firmware_func (void)
 		"<releases>\n"
 		"<release version=\"2.0.2\" timestamp=\"1424116753\">\n"
 		"<location>http://www.hughski.com/downloads/colorhug2/firmware/colorhug-2.0.2.cab</location>\n"
-		"<checksum filename=\"colorhug-als-2.0.2.cab\" target=\"container\" type=\"sha1\">0608b5e8169706f1351a6351630503de75939a86</checksum>\n"
+		"<checksum filename=\"colorhug-als-2.0.2.cab\" target=\"container\" type=\"sha1\">7e179f45d2782c3c9744495dd4bbd91ad3d9e841</checksum>\n"
 		"<checksum filename=\"firmware.bin\" target=\"content\" type=\"sha1\">767a8a7b8a7b350b513f57761204b4aaa657aa44</checksum>\n"
 		"<description><p>This unstable release adds the following features:</p>"
 		"<ul><li>Add TakeReadingArray to enable panel latency measurements</li>"
@@ -847,14 +847,14 @@ asb_test_firmware_func (void)
 		"</release>\n"
 		"<release version=\"2.0.1\" timestamp=\"1424116753\">\n"
 		"<location>http://www.hughski.com/downloads/colorhug2/firmware/colorhug-2.0.1.cab</location>\n"
-		"<checksum filename=\"colorhug-als-2.0.1.cab\" target=\"container\" type=\"sha1\">181496f09df2f09c265742c0e5d9a27fb62b4af7</checksum>\n"
+		"<checksum filename=\"colorhug-als-2.0.1.cab\" target=\"container\" type=\"sha1\">1f8c1a3f92194ebad984df61a6d0c3c1572c1874</checksum>\n"
 		"<checksum filename=\"firmware.bin\" target=\"content\" type=\"sha1\">767a8a7b8a7b350b513f57761204b4aaa657aa44</checksum>\n"
 		"<description><p>This unstable release adds the following features:</p>"
 		"<ul><li>Use TakeReadings() to do a quick non-adaptive measurement</li>"
 		"<li>Scale XYZ measurement with a constant factor to make the CCMX more "
 		"sane</li></ul></description>\n"
 		"<size type=\"installed\">14</size>\n"
-		"<size type=\"download\">1949</size>\n"
+		"<size type=\"download\">1951</size>\n"
 		"</release>\n"
 		"</releases>\n"
 		"<provides>\n"
diff --git a/libappstream-glib/as-self-test.c b/libappstream-glib/as-self-test.c
index c9cd090..dd10aa2 100644
--- a/libappstream-glib/as-self-test.c
+++ b/libappstream-glib/as-self-test.c
@@ -383,7 +383,7 @@ as_test_release_func (void)
 
 	/* verify converting hex prefix */
 	as_release_set_version (release, "0x600100");
-	g_assert_cmpstr (as_release_get_version (release), ==, "96.1.0");
+	g_assert_cmpstr (as_release_get_version (release), ==, "0.96.256");
 }
 
 static void
@@ -1890,7 +1890,7 @@ as_test_app_parse_file_inf_func (void)
 	g_assert_cmpint (releases->len, ==, 1);
 	rel = g_ptr_array_index (releases, 0);
 	g_assert_cmpint (as_release_get_timestamp (rel), ==, 1425340800);
-	g_assert_cmpstr (as_release_get_version (rel), ==, "2.0.2");
+	g_assert_cmpstr (as_release_get_version (rel), ==, "0.2.2");
 	//g_assert_cmpstr (as_release_get_description (rel), ==, "XXX");
 }
 
@@ -2884,7 +2884,7 @@ as_test_store_cab_func (void)
 	"<releases>\n"
 	"<release version=\"2.0.2\" timestamp=\"1424116753\">\n"
 	"<location>http://www.hughski.com/downloads/colorhug2/firmware/colorhug-2.0.2.cab</location>\n"
-	"<checksum filename=\"colorhug-als-2.0.2.cab\" target=\"container\" type=\"sha1\">0608b5e8169706f1351a6351630503de75939a86</checksum>\n"
+	"<checksum filename=\"colorhug-als-2.0.2.cab\" target=\"container\" type=\"sha1\">7e179f45d2782c3c9744495dd4bbd91ad3d9e841</checksum>\n"
 	"<checksum filename=\"firmware.bin\" target=\"content\" type=\"sha1\">767a8a7b8a7b350b513f57761204b4aaa657aa44</checksum>\n"
 	"<description><p>This unstable release adds the following features:</p>"
 	"<ul><li>Add TakeReadingArray to enable panel latency measurements</li>"
@@ -3633,12 +3633,18 @@ as_test_utils_version_func (void)
 	struct {
 		guint32 val;
 		const gchar *ver;
+		AsVersionParseFlag flags;
 	} version_from_uint32[] = {
-		{ 0x0,		"0" },
-		{ 0xff,		"255" },
-		{ 0xff01,	"255.1" },
-		{ 0xff0001,	"255.0.1" },
-		{ 0xff000100,	"255.0.1.0" },
+		{ 0x0,		"0.0.0.0",	AS_VERSION_PARSE_FLAG_NONE },
+		{ 0xff,		"0.0.0.255",	AS_VERSION_PARSE_FLAG_NONE },
+		{ 0xff01,	"0.0.255.1",	AS_VERSION_PARSE_FLAG_NONE },
+		{ 0xff0001,	"0.255.0.1",	AS_VERSION_PARSE_FLAG_NONE },
+		{ 0xff000100,	"255.0.1.0",	AS_VERSION_PARSE_FLAG_NONE },
+		{ 0x0,		"0.0.0",	AS_VERSION_PARSE_FLAG_USE_TRIPLET },
+		{ 0xff,		"0.0.255",	AS_VERSION_PARSE_FLAG_USE_TRIPLET },
+		{ 0xff01,	"0.0.65281",	AS_VERSION_PARSE_FLAG_USE_TRIPLET },
+		{ 0xff0001,	"0.255.1",	AS_VERSION_PARSE_FLAG_USE_TRIPLET },
+		{ 0xff000100,	"255.0.256",	AS_VERSION_PARSE_FLAG_USE_TRIPLET },
 		{ NULL,		NULL }
 	};
 	struct {
@@ -3646,10 +3652,10 @@ as_test_utils_version_func (void)
 		const gchar *new;
 	} version_parse[] = {
 		{ "0",		"0" },
-		{ "257",	"1.1" },
+		{ "257",	"0.0.257" },
 		{ "1.2.3",	"1.2.3" },
-		{ "0xff0001",	"255.0.1" },
-		{ "16711681",	"255.0.1" },
+		{ "0xff0001",	"0.255.1" },
+		{ "16711681",	"0.255.1" },
 		{ "dave",	"dave" },
 		{ NULL,		NULL }
 	};
@@ -3657,7 +3663,8 @@ as_test_utils_version_func (void)
 	/* check version conversion */
 	for (i = 0; version_from_uint32[i].ver != NULL; i++) {
 		g_autofree gchar *ver = NULL;
-		ver = as_utils_version_from_uint32 (version_from_uint32[i].val);
+		ver = as_utils_version_from_uint32 (version_from_uint32[i].val,
+						    version_from_uint32[i].flags);
 		g_assert_cmpstr (ver, ==, version_from_uint32[i].ver);
 	}
 
@@ -3968,7 +3975,7 @@ as_test_utils_vercmp_func (void)
 	g_assert_cmpint (as_utils_vercmp ("1.2.3", "1.2.3"), ==, 0);
 
 	/* same, not dotted decimal */
-	g_assert_cmpint (as_utils_vercmp ("1.2.3", "0x10203"), ==, 0);
+	g_assert_cmpint (as_utils_vercmp ("1.2.3", "0x1020003"), ==, 0);
 	g_assert_cmpint (as_utils_vercmp ("0x10203", "0x10203"), ==, 0);
 
 	/* upgrade and downgrade */
diff --git a/libappstream-glib/as-utils.c b/libappstream-glib/as-utils.c
index 339efa4..5621982 100644
--- a/libappstream-glib/as-utils.c
+++ b/libappstream-glib/as-utils.c
@@ -1666,6 +1666,7 @@ as_utils_guid_from_string (const gchar *str)
 /**
  * as_utils_version_from_uint32:
  * @val: A uint32le version number
+ * @flags: flags used for formatting, e.g. %AS_VERSION_PARSE_FLAG_USE_TRIPLET
  *
  * Returns a dotted decimal version string from a 32 bit number.
  *
@@ -1674,27 +1675,19 @@ as_utils_guid_from_string (const gchar *str)
  * Since: 0.5.2
  **/
 gchar *
-as_utils_version_from_uint32 (guint32 val)
+as_utils_version_from_uint32 (guint32 val, AsVersionParseFlag flags)
 {
-	GString *str;
-	gboolean valid = FALSE;
-	guint i;
-	guint8 *tmp = (guint8 *) &val;
-
-	/* create version string */
-	str = g_string_sized_new (13);
-	for (i = 0; i < 4; i++) {
-		if (tmp[3 - i] > 0 || i == 3)
-			valid = TRUE;
-		if (valid)
-			g_string_append_printf (str, "%i.", tmp[3 - i]);
+	if (flags & AS_VERSION_PARSE_FLAG_USE_TRIPLET) {
+		return g_strdup_printf ("%i.%i.%i",
+					(val >> 24) & 0xff,
+					(val >> 16) & 0xff,
+					val & 0xffff);
 	}
-
-	/* delete trailing dot */
-	if (str->len > 0)
-		g_string_truncate (str, str->len - 1);
-
-	return g_string_free (str, FALSE);
+	return g_strdup_printf ("%i.%i.%i.%i",
+				(val >> 24) & 0xff,
+				(val >> 16) & 0xff,
+				(val >> 8) & 0xff,
+				val & 0xff);
 }
 
 /**
@@ -1746,5 +1739,5 @@ as_utils_version_parse (const gchar *version)
 		return g_strdup (version);
 	if (tmp == 0 || tmp < 0xff)
 		return g_strdup (version);
-	return as_utils_version_from_uint32 (tmp);
+	return as_utils_version_from_uint32 (tmp, AS_VERSION_PARSE_FLAG_USE_TRIPLET);
 }
diff --git a/libappstream-glib/as-utils.h b/libappstream-glib/as-utils.h
index 58944e9..bfc4a38 100644
--- a/libappstream-glib/as-utils.h
+++ b/libappstream-glib/as-utils.h
@@ -108,6 +108,20 @@ typedef enum {
 	AS_MARKUP_CONVERT_FLAG_LAST
 } AsMarkupConvertFlag;
 
+/**
+ * AsVersionParseFlag:
+ * @AS_VERSION_PARSE_FLAG_NONE:			No flags set
+ * @AS_VERSION_PARSE_FLAG_USE_TRIPLET:		Use Microsoft-style version numbers
+ *
+ * The flags used when parsing version numbers.
+ **/
+typedef enum {
+	AS_VERSION_PARSE_FLAG_NONE		= 0,
+	AS_VERSION_PARSE_FLAG_USE_TRIPLET	= 1 << 0,
+	/*< private >*/
+	AS_VERSION_PARSE_FLAG_LAST
+} AsVersionParseFlag;
+
 gchar		*as_markup_convert_simple	(const gchar	*markup,
 						 GError		**error);
 gchar		*as_markup_convert		(const gchar	*markup,
@@ -149,7 +163,8 @@ gint		 as_utils_vercmp		(const gchar	*version_a,
 						 const gchar	*version_b);
 gboolean	 as_utils_guid_is_valid		(const gchar	*guid);
 gchar		*as_utils_guid_from_string	(const gchar	*str);
-gchar		*as_utils_version_from_uint32	(guint32	 val);
+gchar		*as_utils_version_from_uint32	(guint32	 val,
+						 AsVersionParseFlag flags);
 gchar		*as_utils_version_parse		(const gchar	*version);
 
 G_END_DECLS