Re: [PATCH v2 2/4] util: virhostcpu: factor out frequency parsing

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

 



On Wed, 2017-12-13 at 17:35 +0100, Pino Toscano wrote:
> > +    if (!prefix) {
> > +        VIR_WARN("Parser for /proc/cpuinfo needs to be adapted for your architecture");
> > +        return 1;
> 
> I'd print the architecture in the warning, so sysadmins can see easily
> which architecture it is, even when looking at logs collected from
> different libvirt installations.

Sure.

> > +    while (fgets(line, sizeof(line), cpuinfo) != NULL) {
> > +        if (!STRPREFIX(line, prefix))
> > +            continue;
> 
> IMHO here it would be a good idea to check that line[strlen(prefix)]
> is either a space or ':', to avoid prefix matching more keys than the
> actual intended one(s) -- something like:
> 
>   char c = line[strlen(prefix)];
>   if (c != ':' && !c_isspace(*str))
>     continue;

We skip the prefix and pass the rest of the line to
virHostCPUParseFrequencyString(), which starts by skipping all
whitespace and then checking the first non-whitespace character
is a semicolon. So I don't see how we could end up matching
anything but the intended line.

-- 
Andrea Bolognani / Red Hat / Virtualization

--
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