Re: [PATCH v3 4/5] util: Improve CPU frequency parsing

[Date Prev][Date Next][Thread Prev][Thread Next][Date Index][Thread Index]

 



Andrea Bolognani <abologna@xxxxxxxxxx> [2017-12-14, 01:34PM +0100]:
> 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@xxxxxxxxxx>
> ---
>  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

Maybe, >0 when the line prefix does not match exactly?

> + *          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++;

You could do *str++ != ':' and save one line.

> +
> +    /* 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@xxxxxxxxxx
> https://www.redhat.com/mailman/listinfo/libvir-list
> 

-- 
IBM Systems
Linux on z Systems & Virtualization Development
------------------------------------------------------------------------
IBM Deutschland
Schönaicher Str. 220
71032 Böblingen
Phone: +49 7031 16 1819
E-Mail: bwalk@xxxxxxxxxx
------------------------------------------------------------------------
IBM Deutschland Research & Development GmbH
Vorsitzende des Aufsichtsrats: Martina Koederitz
Geschäftsführung: Dirk Wittkopp
Sitz der Gesellschaft: Böblingen
Registergericht: Amtsgericht Stuttgart, HRB 243294 

Attachment: signature.asc
Description: PGP signature

--
libvir-list mailing list
libvir-list@xxxxxxxxxx
https://www.redhat.com/mailman/listinfo/libvir-list

[Index of Archives]     [Virt Tools]     [Libvirt Users]     [Lib OS Info]     [Fedora Users]     [Fedora Desktop]     [Fedora SELinux]     [Big List of Linux Books]     [Yosemite News]     [KDE Users]     [Fedora Tools]
  Powered by Linux