[[RFC PATCH] v2 04/14] Explicitly parse version strings read from flash

Samuel Mendoza-Jonas sam at mendozajonas.com
Thu Jan 18 16:05:07 AEDT 2018


Currently the version strings obtained from lib/flash are used as-is for
display only. To usefully compare versions however this strings need to
be separated into at least package name and package version.

On the OpenPOWER platforms from which these versions are obtained there
is a known list of package names, which hostboot_load_versions() now
uses to parse each version string into a new firmware_version struct
containing the name and version strings.

Signed-off-by: Samuel Mendoza-Jonas <sam at mendozajonas.com>
---
 discover/hostboot.c           |  85 +++++++++++++++++++++++++++++----
 lib/flash/flash.c             |   4 +-
 lib/pb-protocol/pb-protocol.c | 108 +++++++++++++++++++++++++++++++++---------
 lib/types/types.h             |  13 ++++-
 ui/ncurses/nc-sysinfo.c       |   6 ++-
 5 files changed, 180 insertions(+), 36 deletions(-)

diff --git a/discover/hostboot.c b/discover/hostboot.c
index bc9a3ba..c63e363 100644
--- a/discover/hostboot.c
+++ b/discover/hostboot.c
@@ -8,19 +8,88 @@
 
 #include "hostboot.h"
 
+/* Check each version string against a list of known package names and split
+ * them into separate name and version strings.
+ * In a nicer world we would just split based upon a known delimeter but version
+ * strings on OpenPOWER platforms are not consistent.
+ */
+static int parse_hostboot_versions(void *ctx, char **strings,
+		unsigned int n_str, struct firmware_version **versions)
+{
+	const char * cmp_str[] = {"open-power", "buildroot", "skiboot",
+		"hostboot-binaries", "hostboot", "linux", "petitboot", "occ",
+		"capp-ucode", "machine-xml"};
+	unsigned int n_names = sizeof(cmp_str) / sizeof(cmp_str[0]);
+	struct firmware_version *tmp = NULL;
+	unsigned int i, j, n_versions = 0;
+	bool found;
+	char *sep;
+
+	for (i = 0; i < n_str; i++) {
+		found = false;
+		for (j = 0; j < n_names; j++) {
+			if (strncmp(strings[i], cmp_str[j],
+						strlen(cmp_str[j])) == 0) {
+				found = true;
+				break;
+			}
+		}
+		if (!found) {
+			pb_log("Unrecognised version in flash: %s\n",
+					strings[i]);
+			continue;
+		}
+
+		sep = strings[i] + strlen(cmp_str[j]);
+		*sep = '\0';
+		tmp = talloc_realloc(ctx, tmp, struct firmware_version,
+				n_versions + 1);
+		if (!tmp)
+			return -ENOMEM;
+		tmp[n_versions].name = talloc_strdup(tmp, strings[i]);
+		tmp[n_versions].version = talloc_strdup(tmp, sep + 1);
+		n_versions++;
+	}
+
+	*versions = tmp;
+	return n_versions;
+}
+
 void hostboot_load_versions(struct system_info *info)
 {
+	char **strings = NULL;
 	int n = 0;
 
-	n = flash_parse_version(info, &info->platform_primary, true);
-	if (n < 0)
+	n = flash_parse_version(info, &strings, true);
+	if (n < 0) {
 		pb_log("Failed to read platform versions for current side\n");
-	else
-		info->n_primary = n;
+		return;
+	}
 
-	n = flash_parse_version(info, &info->platform_other, false);
-	if (n < 0)
+	n = parse_hostboot_versions(info, strings, n, &info->platform_primary);
+	if (n < 0) {
+		pb_log("Failed to parse platform versions for current side\n");
+		return;
+	}
+	info->n_primary = n;
+
+	n = 0;
+	talloc_free(strings);
+	strings = NULL;
+
+	n = flash_parse_version(info, &strings, false);
+	if (n < 0) {
 		pb_log("Failed to read platform versions for other side\n");
-	else
-		info->n_other = n;
+		return;
+	}
+
+	n = parse_hostboot_versions(info, strings, n, &info->platform_other);
+	if (n < 0) {
+		pb_log("Failed to parse platform versions for current side\n");
+		return;
+	}
+	info->n_other = n;
+
+
+	talloc_free(strings);
 }
diff --git a/lib/flash/flash.c b/lib/flash/flash.c
index 464b876..2322da6 100644
--- a/lib/flash/flash.c
+++ b/lib/flash/flash.c
@@ -190,7 +190,7 @@ int flash_parse_version(void *ctx, char ***versions, bool current)
 			pb_log("%s: Failed to allocate memory\n", __func__);
 			goto out;
 		}
-		tmp[n++] = talloc_strdup(ctx, tok);
+		tmp[n++] = talloc_strdup(tmp, tok);
 	}
 
 	tok = strtok_r(NULL, delim, &saveptr);
@@ -202,7 +202,7 @@ int flash_parse_version(void *ctx, char ***versions, bool current)
 			n = 0;
 			goto out;
 		}
-		tmp[n++] = talloc_strdup(ctx, tok + 1);
+		tmp[n++] = talloc_strdup(tmp, tok + 1);
 		tok = strtok_r(NULL, delim, &saveptr);
 	}
 
diff --git a/lib/pb-protocol/pb-protocol.c b/lib/pb-protocol/pb-protocol.c
index dbbda40..76a2665 100644
--- a/lib/pb-protocol/pb-protocol.c
+++ b/lib/pb-protocol/pb-protocol.c
@@ -226,6 +226,21 @@ int pb_protocol_boot_status_len(const struct status *status)
 		4;
 }
 
+static int pb_protocol_firmware_version_len(
+		const struct firmware_version *versions,
+		unsigned int n_versions)
+{
+	unsigned int i;
+	int len = 0;
+
+	for (i = 0; i < n_versions; i++) {
+		len += 4 + optional_strlen(versions[i].name);
+		len += 4 + optional_strlen(versions[i].version);
+	}
+
+	return len;
+}
+
 int pb_protocol_system_info_len(const struct system_info *sysinfo)
 {
 	unsigned int len, i;
@@ -234,12 +249,15 @@ int pb_protocol_system_info_len(const struct system_info *sysinfo)
 		4 + optional_strlen(sysinfo->identifier) +
 		4 + 4;
 
-	len +=	4;
-	for (i = 0; i < sysinfo->n_primary; i++)
-		len += 4 + optional_strlen(sysinfo->platform_primary[i]);
-	len +=	4;
-	for (i = 0; i < sysinfo->n_other; i++)
-		len += 4 + optional_strlen(sysinfo->platform_other[i]);
+	len +=  4 /* n_primary */;
+	len += pb_protocol_firmware_version_len(sysinfo->platform_primary,
+			sysinfo->n_primary);
+	len +=  4 /* n_other */;
+	len += pb_protocol_firmware_version_len(sysinfo->platform_other,
+			sysinfo->n_other);
+	len +=  4 /* n_new */;
+	len += pb_protocol_firmware_version_len(sysinfo->platform_new,
+			sysinfo->n_new);
 
 	len +=	4;
 	for (i = 0; i < sysinfo->n_bmc_current; i++)
@@ -447,6 +465,21 @@ int pb_protocol_serialise_boot_status(const struct status *status,
 	return 0;
 }
 
+static int pb_protocol_serialse_firmware_version(
+		const struct firmware_version *versions,
+		unsigned int n_versions, char *buf)
+{
+	char *pos = buf;
+	unsigned int i;
+
+	for (i = 0; i < n_versions; i++) {
+		pos += pb_protocol_serialise_string(pos, versions[i].name);
+		pos += pb_protocol_serialise_string(pos, versions[i].version);
+	}
+
+	return pos - buf;
+}
+
 int pb_protocol_serialise_system_info(const struct system_info *sysinfo,
 		char *buf, int buf_len)
 {
@@ -458,13 +491,18 @@ int pb_protocol_serialise_system_info(const struct system_info *sysinfo,
 
 	*(uint32_t *)pos = __cpu_to_be32(sysinfo->n_primary);
 	pos += sizeof(uint32_t);
-	for (i = 0; i < sysinfo->n_primary; i++)
-		pos += pb_protocol_serialise_string(pos, sysinfo->platform_primary[i]);
+	pos += pb_protocol_serialse_firmware_version(
+			sysinfo->platform_primary, sysinfo->n_primary, pos);
 
 	*(uint32_t *)pos = __cpu_to_be32(sysinfo->n_other);
 	pos += sizeof(uint32_t);
-	for (i = 0; i < sysinfo->n_other; i++)
-		pos += pb_protocol_serialise_string(pos, sysinfo->platform_other[i]);
+	pos += pb_protocol_serialse_firmware_version(
+			sysinfo->platform_other, sysinfo->n_other, pos);
+
+	*(uint32_t *)pos = __cpu_to_be32(sysinfo->n_new);
+	pos += sizeof(uint32_t);
+	pos += pb_protocol_serialse_firmware_version(
+			sysinfo->platform_new, sysinfo->n_new, pos);
 
 	*(uint32_t *)pos = __cpu_to_be32(sysinfo->n_bmc_current);
 	pos += sizeof(uint32_t);
@@ -921,6 +959,25 @@ out:
 	return rc;
 }
 
+static int pb_protocol_deserialise_firmware_version(const char **buf,
+		unsigned int *len, struct firmware_version *versions,
+		unsigned int n_versions)
+{
+	unsigned int i;
+	char *tmp;
+
+	for (i = 0; i < n_versions; i++) {
+		if (read_string(versions, buf, len, &tmp))
+			return -1;
+		versions[i].name = talloc_strdup(versions, tmp);
+		if (read_string(versions, buf, len, &tmp))
+			return -1;
+		versions[i].version = talloc_strdup(versions, tmp);
+	}
+
+	return 0;
+}
+
 int pb_protocol_deserialise_system_info(struct system_info *sysinfo,
 		const struct pb_protocol_message *message)
 {
@@ -942,23 +999,30 @@ int pb_protocol_deserialise_system_info(struct system_info *sysinfo,
 	/* Platform version strings for openpower platforms */
 	if (read_u32(&pos, &len, &sysinfo->n_primary))
 		goto out;
-	sysinfo->platform_primary = talloc_array(sysinfo, char *,
+	sysinfo->platform_primary = talloc_array(sysinfo,
+						struct firmware_version,
 						sysinfo->n_primary);
-	for (i = 0; i < sysinfo->n_primary; i++) {
-		if (read_string(sysinfo, &pos, &len, &tmp))
-			goto out;
-		sysinfo->platform_primary[i] = talloc_strdup(sysinfo, tmp);
-	}
+	if (pb_protocol_deserialise_firmware_version(&pos, &len,
+				sysinfo->platform_primary, sysinfo->n_primary))
+		goto out;
 
 	if (read_u32(&pos, &len, &sysinfo->n_other))
 		goto out;
-	sysinfo->platform_other = talloc_array(sysinfo, char *,
+	sysinfo->platform_other = talloc_array(sysinfo,
+						struct firmware_version,
 						sysinfo->n_other);
-	for (i = 0; i < sysinfo->n_other; i++) {
-		if (read_string(sysinfo, &pos, &len, &tmp))
-			goto out;
-		sysinfo->platform_other[i] = talloc_strdup(sysinfo, tmp);
-	}
+	if (pb_protocol_deserialise_firmware_version(&pos, &len,
+				sysinfo->platform_other, sysinfo->n_other))
+		goto out;
+
+	if (read_u32(&pos, &len, &sysinfo->n_new))
+		goto out;
+	sysinfo->platform_new = talloc_array(sysinfo,
+						struct firmware_version,
+						sysinfo->n_new);
+	if (pb_protocol_deserialise_firmware_version(&pos, &len,
+				sysinfo->platform_new, sysinfo->n_new))
+		goto out;
 
 	/* BMC version strings for openpower platforms */
 	if (read_u32(&pos, &len, &sysinfo->n_bmc_current))
diff --git a/lib/types/types.h b/lib/types/types.h
index 9ab2a43..f4312e6 100644
--- a/lib/types/types.h
+++ b/lib/types/types.h
@@ -118,13 +118,22 @@ struct blockdev_info {
 	char		*mountpoint;
 };
 
+struct firmware_version {
+	char		*name;
+	char		*version;
+};
+
 struct system_info {
 	char			*type;
 	char			*identifier;
-	char			**platform_primary;
-	char			**platform_other;
+
+	struct firmware_version *platform_primary;
 	unsigned int		n_primary;
+	struct firmware_version *platform_other;
 	unsigned int		n_other;
+	struct firmware_version *platform_new;
+	unsigned int		n_new;
+
 	char			**bmc_current;
 	char			**bmc_golden;
 	unsigned int		n_bmc_current;
diff --git a/ui/ncurses/nc-sysinfo.c b/ui/ncurses/nc-sysinfo.c
index 8252b45..2d3ce82 100644
--- a/ui/ncurses/nc-sysinfo.c
+++ b/ui/ncurses/nc-sysinfo.c
@@ -69,7 +69,8 @@ static void sysinfo_screen_populate(struct sysinfo_screen *screen,
 		line(NULL);
 		line("%s", _("Primary platform versions:"));
 		for (i = 0; i < sysinfo->n_primary; i++) {
-			line("\t%s", sysinfo->platform_primary[i] ?: "");
+			line("\t%s: %s", sysinfo->platform_primary[i].name,
+					sysinfo->platform_primary[i].version);
 		}
 	}
 
@@ -77,7 +78,8 @@ static void sysinfo_screen_populate(struct sysinfo_screen *screen,
 		line(NULL);
 		line("%s", _("Alternate platform versions:"));
 		for (i = 0; i < sysinfo->n_other; i++) {
-			line("\t%s", sysinfo->platform_other[i] ?: "");
+			line("\t%s: %s", sysinfo->platform_other[i].name,
+					sysinfo->platform_other[i].version);
 		}
 	}
 
-- 
2.15.1



More information about the Petitboot mailing list