Pino Toscano <[email protected]> [2017-12-13, 05:35PM +0100]: > On Wednesday, 13 December 2017 17:10:19 CET Andrea Bolognani wrote: > > From: Bjoern Walk <[email protected]> > > > > All different architectures use the same copy-pasted code to parse > > processor frequency information from /proc/cpuinfo. Let's extract that > > code into a function to avoid repetition. > > > > We now also tolerate if the parsing of /proc/cpuinfo is not successful > > and just report a warning instead of bailing out and abandoning the rest > > of the CPU information. > > > > Reviewed-by: Marc Hartmayer <[email protected]> > > Reviewed-by: Boris Fiuczynski <[email protected]> > > Reviewed-by: Andrea Bolognani <[email protected]> > > Signed-off-by: Bjoern Walk <[email protected]> > > --- > > [...] > > +static int > > +virHostCPUParseFrequency(FILE *cpuinfo, > > + virArch arch, > > + unsigned int *mhz) > > +{ > > + const char *prefix = NULL; > > + char line[1024]; > > + > > + if (ARCH_IS_X86(arch)) > > + prefix = "cpu MHz"; > > + else if (ARCH_IS_PPC(arch)) > > + prefix = "clock"; > > + else if (ARCH_IS_ARM(arch)) > > + prefix = "BogoMIPS"; > > + > > + 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. >
That's probably a good idea.
> > + 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;
>
Here I'm not sure. This would be for optimization, right? Because we
have this check in the parsing function. I don't think that this code is
so hard-pressed for optimizations that we should make it more
complicated. But I have no hard feelings over this, it's Andrea's call.
> --
> Pino Toscano
> --
> libvir-list mailing list
> [email protected]
> 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: [email protected]
------------------------------------------------------------------------
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
signature.asc
Description: PGP signature
-- libvir-list mailing list [email protected] https://www.redhat.com/mailman/listinfo/libvir-list
