Re: [libvirt] [PATCH v3 4/5] 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(a)redhat.com&gt; --- src/util/virhostcpu.c | 62 +++++++++++++++++++++---- tests/virhostcpudata/linux-x86_64-test1.cpuinfo | 4 ++ 2 files changed, 57 insertions(+), 9 deletions(-) diff --git a/src/util/virhostcpu.c b/src/util/virhostcpu.c index 3c20755eb..9c9f362de 100644 --- a/src/util/virhostcpu.c +++ b/src/util/virhostcpu.c @@ -508,6 +508,27 @@ virHostCPUHasValidSubcoreConfiguration(int threads_per_subcore) return ret; } +/** + * 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, @@ -516,26 +537,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; } static int diff --git a/tests/virhostcpudata/linux-x86_64-test1.cpuinfo b/tests/virhostcpudata/linux-x86_64-test1.cpuinfo index e88a48ff3..706b69a54 100644 --- 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 -- 2.14.3 -- libvir-list mailing list libvir-list(a)redhat.com https://www.redhat.com/mailman/listinfo/libvir-list