If nstrings is a uint8_t, how can we support 99999 OEM strings?

Why not report an error instead?

Again, why not report malformed input instead of silently ignoring it?

Why not report this case as an error?

Better to use calloc().

template_strings doesn't get freed anywhere.

To me this is clearer as

for (i = 0; template_strings[i].node != NULL; i++)
    ;

Spurious change.

Hmm. Going back over this, the DMTF spec defines the header as:
04h | Count | BYTE | Varies Number of strings
so the uint8_t is correct, and the 99,999 number used later is wrong. It appears qemu allows configuring an arbitrary number of these strings but ultimately truncates the count to an 8 bit value.

I'm going to leave this as-is but change the subsequent checks to 255 instead of 99999.

The memory contains both the "array" of struct smbios_string plus the strings themselves immediately following the structs. calloc() didn't seem like the obvious choice here, but perhaps I'm missing something?

I think this is overly complicated. You can instead:

• Loop over the keys, validate them and the values. The values are always strings. Keep track of the largest key.
• Put s = calloc((maxnumber + 1), sizeof(struct smbios_string)).
• Loop over the keys again. Set s[key].node = keystr; s[key].value = strdup(value) where keystr is allocated with something like asprintf(&str, "%05d", key).
• For table entries without a corresponding value, set s[i] = "N/A"; or whatever the default value should be.

Then you don't need the extra nvlist or any fragile logic to figure out how big a buffer to allocate.

Please use the BHYVE_EXIT_* codes from bhyverun.h, here and below.

You don't need a flag variable, just

for (i = 0; name[i] != '\0'; i++)
    if (!isdigit(name[i]))
        errx(BHYVE_EXIT_ERROR, ...);

You also need to check whether *np == '\0'.

If the key has leading zeros it'll be parsed as an octal number, is that desired? If not, the base should be specified.

I do not think the type can be anything other than NV_TYPE_STRING.

numberkey isn't guaranteed to be initialized here.

I'm OK with this approach, but then, is it acceptable to use a double-cast to free the memory pointed to by .node and .value as struct smbios_string defines them as const char *? E.g.,

for (i = 0; type11_strings[i].node != NULL; i++) {
	free((void *)(uintptr_t)type11_strings[i].node);
	free((void *)(uintptr_t)type11_strings[i].value);
}

That's fine here, you can use __DECONST to make it more explicit.

It is only to make the compiler happy on two fronts:

1. Using it allows freeing the memory returned from smbios_gather_type11_strings() without using __DECONST()
2. By assigning it to template_strings, we avoid an unused parameter error.

Would it be preferable to drop type11_strings and assign the gathered strings to template_strings and then use __DECONST() to free the memory?

I don't understand the first point, below you're freeing type11_strings, not template_strings.

The second point can be resolved by annotating the parameter with __unused.