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

[Pino Toscano]

On Thursday, 14 December 2017 10:46:33 CET Andrea Bolognani wrote:
> On Wed, 2017-12-13 at 17:35 +0100, Pino Toscano wrote:
> > > +    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.

Ah sorry, I did not explain all: the situation I see is that
virHostCPUParseFrequencyString errors out if it does not find the
colon.  Let's say that on x86_64 /proc/cpuinfo contains:

  cpu MHz new     : 1000.000
  cpu MHz         : 2000.000

since "cpu MHz" is the prefix on x86, then the "cpu MHz new" line
matches it so virHostCPUParseFrequencyString will be called, but then
virHostCPUParseFrequencyString will error out because (after skipping
spaces) it will find 'n'.  A failure in virHostCPUParseFrequencyString
is propagated directly by virHostCPUParseFrequency, so the real key in
cpuinfo will not be read.

-- 
Pino Toscano
-------------- next part --------------
A non-text attachment was scrubbed...
Name: signature.asc
Type: application/pgp-signature
Size: 833 bytes
Desc: This is a digitally signed message part.
URL: <http://listman.redhat.com/archives/libvir-list/attachments/20171214/79676376/attachment-0001.sig>