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

base/cpu.cc

 // Copyright (c) 2012 The Chromium Authors. All rights reserved.
 // Use of this source code is governed by a BSD-style license that can be
 // found in the LICENSE file.
 
 #include "base/cpu.h"
 
 #include <string.h>
 
 #include "build/build_config.h"
 
@@ skipping 66 matching lines @@
   );
 }
 
 #endif
 #endif  // _MSC_VER
 #endif  // ARCH_CPU_X86_FAMILY
 
 void CPU::Initialize() {
 #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
   // the other three array elements. The CPU identification string is
   // not in linear order. The code below arranges the information
   // in a human readable form.
   //
   // More info can be found here:
   // http://msdn.microsoft.com/en-us/library/hskdteyh.aspx
   __cpuid(cpu_info, 0);
   int num_ids = cpu_info[0];
   memset(cpu_string, 0, sizeof(cpu_string));
   *(reinterpret_cast<int*>(cpu_string)) = cpu_info[1];
   *(reinterpret_cast<int*>(cpu_string+4)) = cpu_info[3];

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

nit: white space around operator "+".

Better yet IMO would be to use a subscript operator (as suggested for the new
code).

   *(reinterpret_cast<int*>(cpu_string+8)) = cpu_info[2];
 
   // Interpret CPU feature information.
   if (num_ids > 0) {
     __cpuid(cpu_info, 1);
     stepping_ = cpu_info[0] & 0xf;
     model_ = ((cpu_info[0] >> 4) & 0xf) + ((cpu_info[0] >> 12) & 0xf0);
     family_ = (cpu_info[0] >> 8) & 0xf;
     type_ = (cpu_info[0] >> 12) & 0x3;
     ext_model_ = (cpu_info[0] >> 16) & 0xf;
     ext_family_ = (cpu_info[0] >> 20) & 0xff;
     cpu_vendor_ = cpu_string;
     has_mmx_ = (cpu_info[3] & 0x00800000) != 0;
     has_sse_ = (cpu_info[3] & 0x02000000) != 0;
     has_sse2_ = (cpu_info[3] & 0x04000000) != 0;
     has_sse3_ = (cpu_info[2] & 0x00000001) != 0;
     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
 }
 
 }  // namespace base