util: Improve CPU frequency parsing

Make the parser both more strict, by not ignoring errors reported
by virStrToLong_ui(), and more permissive, by not failing due to
unrelated fields which just happen to have a know prefix and
accepting any amount of whitespace before the numeric value.

Signed-off-by: Andrea Bolognani <abologna@redhat.com>
Reviewed-by: Bjoern Walk <bwalk@linux.vnet.ibm.com>

diff --git a/src/util/virhostcpu.c b/src/util/virhostcpu.c
index fd9fec870c0deb6091ab15c0c497051ecb3d2f53..f3ee3a1a58e9afc847cbb9ef21b673471c885911 100644 (file)
--- a/src/util/virhostcpu.c
+++ b/src/util/virhostcpu.c
@@ -509,6 +509,27 @@ virHostCPUHasValidSubcoreConfiguration(int threads_per_subcore)
 }
 
 
+/**
+ * virHostCPUParseFrequencyString:
+ * @str: string to be parsed
+ * @prefix: expected prefix
+ * @mhz: output location
+ *
+ * Parse a /proc/cpuinfo line and extract the CPU frequency, if present.
+ *
+ * The expected format of @str looks like
+ *
+ *   cpu MHz : 2100.000
+ *
+ * where @prefix ("cpu MHz" in the example), is architecture-dependent.
+ *
+ * The decimal part of the CPU frequency, as well as all whitespace, is
+ * ignored.
+ *
+ * Returns: 0 when the string has been parsed successfully and the CPU
+ *          frequency has been stored in @mhz, >0 when the string has not
+ *          been parsed but no error has occurred, <0 on failure.
+ */
 static int
 virHostCPUParseFrequencyString(const char *str,
                                const char *prefix,
@@ -517,26 +538,49 @@ virHostCPUParseFrequencyString(const char *str,
     char *p;
     unsigned int ui;
 
+    /* If the string doesn't start with the expected prefix, then
+     * we're not looking at the right string and we should move on */
     if (!STRPREFIX(str, prefix))
         return 1;
 
+    /* Skip the prefix */
     str += strlen(prefix);
 
-    while (*str && c_isspace(*str))
+    /* Skip all whitespace */
+    while (c_isspace(*str))
         str++;
+    if (*str == '\0')
+        goto error;
 
-    if (*str != ':' || !str[1]) {
-        virReportError(VIR_ERR_INTERNAL_ERROR, "%s",
-                       _("parsing cpu MHz from cpuinfo"));
-        return -1;
+    /* Skip the colon. If anything but a colon is found, then we're
+     * not looking at the right string and we should move on */
+    if (*str != ':')
+        return 1;
+    str++;
+
+    /* Skip all whitespace */
+    while (c_isspace(*str))
+        str++;
+    if (*str == '\0')
+        goto error;
+
+    /* Parse the frequency. We expect an unsigned value, optionally
+     * followed by a fractional part (which gets discarded) or some
+     * leading whitespace */
+    if (virStrToLong_ui(str, &p, 10, &ui) < 0 ||
+        (*p != '.' && *p != '\0' && !c_isspace(*p))) {
+        goto error;
     }
 
-    if (virStrToLong_ui(str + 1, &p, 10, &ui) == 0 &&
-        /* Accept trailing fractional part. */
-        (*p == '\0' || *p == '.' || c_isspace(*p)))
-        *mhz = ui;
+    *mhz = ui;
 
     return 0;
+
+ error:
+    virReportError(VIR_ERR_INTERNAL_ERROR,
+                   _("Missing or invalid CPU frequency in %s"),
+                   CPUINFO_PATH);
+    return -1;
 }
 
 

diff --git a/tests/virhostcpudata/linux-x86_64-test1.cpuinfo b/tests/virhostcpudata/linux-x86_64-test1.cpuinfo
index e88a48ff35fb420dc9fc31e84263f9385cf75eef..706b69a54396f56509adf9b6199d3965395e1101 100644 (file)
--- a/tests/virhostcpudata/linux-x86_64-test1.cpuinfo
+++ b/tests/virhostcpudata/linux-x86_64-test1.cpuinfo
@@ -28,6 +28,10 @@ model                : 4
 model name     :                   Intel(R) Xeon(TM) CPU 2.80GHz
 stepping       : 8
 cpu MHz                : 2800.000
+# The following field is a made-up one, intended to make sure our cpuinfo
+# parser deals correctly with the introduction of new fields that just so
+# happen to share a prefix with the one used for CPU frequency
+cpu MHz rounded up to GHz              : 3000.000
 cache size     : 2048 KB
 physical id    : 0
 siblings       : 2