Use the cpuid instruction to generate the CPU brand string. And

base/cpu.cc

Index: base/cpu.cc
===================================================================
--- base/cpu.cc	(revision 159378)
+++ base/cpu.cc	(working copy)
@@ -85,6 +85,7 @@
 #if defined(ARCH_CPU_X86_FAMILY)
   int cpu_info[4] = {-1};
   char cpu_string[0x20];

[jar (doing other things), 2012/10/03 00:33:04]

Here this is declared to be length 32, but on lines 101-103, we only use 12
bytes, and perhaps counting on null trailing bytes at offset 13 to terminate the
string.  IMO, it would be helpful to use decimal, and this might be clearer.

+  char cpu_brand[0x30];

[jar (doing other things), 2012/10/03 00:33:04]

Please use decimal, rather than hex here.  48 is much clearer.

Given that the code writes 48 bytes, it would make sense to add at least one
extra byte with a null, since you're going to assume it is null terminated on
line 137.

 
   // __cpuid with an InfoType argument of 0 returns the number of
   // valid Ids in CPUInfo[0] and the CPU identification string in
@@ -118,6 +119,23 @@
     has_ssse3_ = (cpu_info[2] & 0x00000200) != 0;
     has_sse41_ = (cpu_info[2] & 0x00080000) != 0;
     has_sse42_ = (cpu_info[2] & 0x00100000) != 0;
+
+    // get the brand string of the cpu.
+    __cpuid(cpu_info, 0x80000000);
+
+    if (cpu_info[0] >= 0x80000004) {
+      memset(cpu_brand, 0, sizeof(cpu_brand));
+      int start_flag = 0x80000002;

[Mark Mentovai, 2012/10/03 00:33:17]

“flag” isn’t a good name. Thcpuid documentation usually calls the eax parameter
to cpuid a “function,” but “parameter” or “argument” would be acceptable too.

+
+      for (int index = 0; index < 3; index++) {

[Mark Mentovai, 2012/10/03 00:33:17]

Why don’t you just write the loop by initializing the loop variable to
0x80000002 and testing that it <= 0x80000004?

Then, 0x80000004 can be a const int, and you can use it in the check on line 126
as well.

+        __cpuid(cpu_info, start_flag + index);
+        *(reinterpret_cast<int*>(cpu_brand + index*16)) = cpu_info[0];

[jar (doing other things), 2012/10/03 00:33:04]

nit: spaces around an arithmetic operator "*"

Better yet IMO, would be to use the subscript operator, which is much clearer
than pointer arithmetic.

See comment below for having the nicer choice of types.

You could probably also do this nicer with a nested loop, rather than repeating
the same line (almost) on lines 132-135.

Note that on lines 101-103 they are doing some permuting (perhaps
bigendian/littleendian isuses??), but you're just copying in order here (I
think).

[Mark Mentovai, 2012/10/03 00:33:17]

I can see that this was copied from line 101, but this isn’t great. This type
punning breaks strict aliasing rules. It’s also a little silly to do these four
operations when a single memcpy would suffice. memcpy is also fine even under
strict aliasing rules.

You should fix up the other code in this file as well.

+        *(reinterpret_cast<int*>(cpu_brand + index*16 + 4)) = cpu_info[1];
+        *(reinterpret_cast<int*>(cpu_brand + index*16 + 8)) = cpu_info[2];
+        *(reinterpret_cast<int*>(cpu_brand + index*16 + 12)) = cpu_info[3];
+      }
+      cpu_brand_ = cpu_brand;

[jar (doing other things), 2012/10/03 00:33:04]

How about declaring cpu_brand to be:
int cup_brand[12];
Then you'd only have one (sad) cast:
cpu_brand_ = reinterpret_cast<char*>(cpu_brand);

+    }
   }
 #endif
 }