Is this CPUID parser ideal for any usage?

NOTE: I'm not perfectly sure of some of the parsed data, so any corrections are more than welcome.

Parser (cpuid.c):

```
#include
#include

#include "cpuid.h"

enum {
CPU_PROC_BRAND_STRING_INTERNAL0 = 0x80000003,
CPU_PROC_BRAND_STRING_INTERNAL1 = 0x80000004
};

#ifndef _MSC_VER
static void ___cpuid(cpuid_t info, uint32_t eax, uint32_t ebx, uint32_t ecx, uint32_t edx)
{
__asm__(
"xchg %%ebx, %%edi\n\t" / 32bit PIC: Don't clobber ebx. /
"cpuid\n\t"
"xchg %%ebx, %%edi\n\t"
: "=a"(eax), "=D"(ebx), "=c"(ecx), "=d"(edx)
: "0" (info)
);
}
#else
#include

static void ___cpuid(cpuid_t info, uint32_t eax, uint32_t ebx, uint32_t ecx, uint32_t edx)
{
uint32_t registers[4];
__cpuid(registers, info);

*eax = registers[0];
*ebx = registers[1];
*ecx = registers[2];
*edx = registers[3];
}
#endif

int highest_ext_func_supported(void)
{
static int highest;

if (!highest) {
asm volatile(
"cpuid\n\t"
: "=a" (highest)
: "a" (CPU_HIGHEST_EXTENDED_FUNCTION_SUPPORTED)
);
}

return highest;
}

int cpuid_test_feature(cpuid_t feature)
{
if (feature > CPU_VIRT_PHYS_ADDR_SIZES || feature CPU_VIRT_PHYS_ADDR_SIZES || (info > CPU_HIGHEST_EXTENDED_FUNCTION_SUPPORTED
&& !cpuid_test_feature(info)))
return;

uint32_t *ubuf = buf;
if (info == CPU_PROC_BRAND_STRING) {
___cpuid(CPU_PROC_BRAND_STRING, &ubuf[0], &ubuf[1], &ubuf[2], &ubuf[3]);
___cpuid(CPU_PROC_BRAND_STRING_INTERNAL0, &ubuf[4], &ubuf[5], &ubuf[6], &ubuf[7]);
___cpuid(CPU_PROC_BRAND_STRING_INTERNAL1, &ubuf[8], &ubuf[9], &ubuf[10], &ubuf[11]);
return;
} else if (info == CPU_HIGHEST_EXTENDED_FUNCTION_SUPPORTED) {
*ubuf = highest_ext_func_supported();
return;
}

uint32_t eax, ebx, ecx, edx;
___cpuid(info, &eax, &ebx, &ecx, &edx);

switch (info) {
cas

Here are some comments on your code.

I think your types cpufeature_t and cpuextfeature_t are probably
unnecessary as they are just uint32_t. They might be better expressed as
enums.

Functions that are purely for local use (ie. not part of the public interface)
should be static. This prevents polluting the global namespace and allows
better optimisation.

Function ___cpuid should be renamed without the underscores (which are
reserved for use by the implementation). A better name might be get_cpuid.
The function also takes a cpuid_t parameter named info that might be
better named request.

In cpuid_has_feature the switch should really have a default case,
although the trailing return 0 kind of takes its place. The casts to int
should be to uint32_t instead to match the types of eax etc. Note also
that

return (edx & ((uint32_t)feature)) != 0;

is the same as

return edx & (uint32_t)feature;

In cpuid_has_ext_feature, the initial call to
cpuid_test_feature(CPU_EXTENDED_PROC_INFO_FEATURE_BITS) is unnecessary
because you already know that CPU_EXTENDED_PROC_INFO_FEATURE_BITS is valid.
The switch default is also missing.

In get_cpu_type you return an integer index into your local array of ID
strings but that index has no meaning to the caller unless they have exacctly
the same list of ID strings. Except for the first entry, your list matches
that in Wikipedia, but that is rather weak. I'd prefer to see an enum or #defined constant associated with each array entry.

Also in that function you terminate the string u.buf[12] = '\0'; and in
doing so break the stack (u.buf has a size of 12 so you wrote beyond the
end). You don't actually need to terminate as you use strncmp for your
string comparisons - note that the explicit 12 in the strncmp should really
be sizeof cpuids[0].

In cpuid I don't like the use of a void* parameter to return the various
types of information (strings, integers etc). A better solution would be to
pass a pointer to a structure or a discriminated union that has a field for
each type of information returned by cpuid.

StackExchange Code Review Q#31532, answer score: 4