On Tue, Apr 22, 2014 at 06:16:53PM +0200, Markus Armbruster wrote:
smbios_entry_add() gets called for every "-smbios file=..." and "-smbios type=..." command line argument
a. "type=..." is OK, we just remember the values for later
b. "file=..." less so, because we have to load the blobs into *something*.
Remember the filenames for later ...
In my v7 patch set I decided to load the blobs into *both* the legacy and new-fangled aggregate tables.
smbios_set_defaults() is called
- by now we *already* know the machine version we have, so we get to free the table we now know we won't need (aggregate if we're on <= v2.0, legacy if 2.1 or newer, see 1.b. above)
... and load the blobs here?
I don't like this technique, because it tends to degrade error messages. When you're processing the option, your error messages come out with nice location information automatically. If you just store filenames, that's lost. To preserve it, you need to do extra work.
Note that smbios.c already stores the -smbios type=... for later use by smbios_get_table()'s table building. Could you store the blob, too?
Can you all please check out v7 of the patch set at
http://lists.nongnu.org/archive/html/qemu-devel/2014-04/msg03195.html
In particular, patch 7/7 (toward the bottom, where smbios_entry_add() is being patched, here:
http://lists.nongnu.org/archive/html/qemu-devel/2014-04/msg03196.html
The existing code already stores the blobs, wrapped in a SMBIOS_TABLE_ENTRY wrapper, in "smbios_entries".
The new code needs the blobs *without* any wrappers, just concatenated together, in the new aggregate table I named "smbios_tables".
So I'm only reading the file blobs *once* (in the new code), and then malloc room for a wrapper+blob in the legacy smbios_entries table, and memcpy from where I already read them into "smbios_tables".
I think this is the cleanest way to have both legacy and new/aggregate logic live side by side. Intrinsically, the new code wouldn't need to "reuse" any of the old code, so trying to factor out any "common bits" would actually be less clean, IMHO...
Once you've had a chance to look at the latest patch, please let me know if you have any strong objections to how it's handling the problem, or advice on how to do it more cleanly...
Thanks much, --Gabriel