Pino Toscano <ptoscano@xxxxxxxxxx> [2017-12-13, 05:35PM +0100]:
> On Wednesday, 13 December 2017 17:10:19 CET Andrea Bolognani wrote:
> > From: Bjoern Walk <bwalk@xxxxxxxxxxxxxxxxxx>
> >
> > 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 <mhartmay@xxxxxxxxxxxxxxxxxx>
> > Reviewed-by: Boris Fiuczynski <fiuczy@xxxxxxxxxxxxxxxxxx>
> > Reviewed-by: Andrea Bolognani <abologna@xxxxxxxxxx>
> > Signed-off-by: Bjoern Walk <bwalk@xxxxxxxxxxxxxxxxxx>
> > ---
> > [...]
> > +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
> libvir-list@xxxxxxxxxx
> 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: bwalk@xxxxxxxxxx
------------------------------------------------------------------------
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
Attachment:
signature.asc
Description: PGP signature
-- libvir-list mailing list libvir-list@xxxxxxxxxx https://www.redhat.com/mailman/listinfo/libvir-list
