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

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.