On Thu, 2017-12-14 at 11:07 +0100, Pino Toscano wrote:
> 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.
You realize the change you propose[1] wouldn't deal properly with
your very example, right? ;)
I'll come up with something: not that I expect this to cause much
actual harm, but since we're fixing it already might as well go the
extra mile.
[1] Assuming passing '*str' rather than 'c' to c_isspace() is a
mere pasto, which would be consistent with your explanation
of its intended purpose.
--
Andrea Bolognani / Red Hat / Virtualization
--
libvir-list mailing list
libvir-list@redhat.com
https://www.redhat.com/mailman/listinfo/libvir-list
