diff mbox

[3/7] smbios: Improve diagnostics for conflicting entries

Message ID 1374081369-1511-4-git-send-email-armbru@redhat.com
State New
Headers show

Commit Message

Markus Armbruster July 17, 2013, 5:16 p.m. UTC 
We allow either tables or fields for the same type.  Makes sense,
because SeaBIOS uses fields only when no tables are present.

We do this by searching the SMBIOS blob for a previously added table
or field.  Error messages look like this:

    qemu-system-x86_64: -smbios type=1,serial=42: SMBIOS type 1 table already defined, cannot add field

User needs to know that "table" is defined by -smbios file=..., and
"field" by -smbios type=...

Instead of searching the blob, record additions of interest, and check
that.  Simpler, and makes better error messages possible:

    qemu-system-x86_64: -smbios file=smbios_type_1.bin: Can't mix file= and type= for same type
    qemu-system-x86_64: -smbios type=1,serial=42,serial=99: This is the conflicting setting

Signed-off-by: Markus Armbruster <armbru@redhat.com>
---
 hw/i386/smbios.c | 43 +++++++++++++++++--------------------------
 1 file changed, 17 insertions(+), 26 deletions(-)

Comments

Eric Blake July 17, 2013, 7:09 p.m. UTC | #1 
On 07/17/2013 11:16 AM, Markus Armbruster wrote:
> We allow either tables or fields for the same type.  Makes sense,
> because SeaBIOS uses fields only when no tables are present.
> 
> We do this by searching the SMBIOS blob for a previously added table
> or field.  Error messages look like this:
> 
>     qemu-system-x86_64: -smbios type=1,serial=42: SMBIOS type 1 table already defined, cannot add field
> 
> User needs to know that "table" is defined by -smbios file=..., and
> "field" by -smbios type=...
> 
> Instead of searching the blob, record additions of interest, and check
> that.  Simpler, and makes better error messages possible:
> 
>     qemu-system-x86_64: -smbios file=smbios_type_1.bin: Can't mix file= and type= for same type
>     qemu-system-x86_64: -smbios type=1,serial=42,serial=99: This is the conflicting setting
> 
> Signed-off-by: Markus Armbruster <armbru@redhat.com>
> ---
>  hw/i386/smbios.c | 43 +++++++++++++++++--------------------------
>  1 file changed, 17 insertions(+), 26 deletions(-)

Nicer message in fewer lines of code - what's not to like about it?

Reviewed-by: Eric Blake <eblake@redhat.com>
diff mbox

Patch

diff --git a/hw/i386/smbios.c b/hw/i386/smbios.c
index a113f8b..7908c47 100644
--- a/hw/i386/smbios.c
+++ b/hw/i386/smbios.c
@@ -48,6 +48,12 @@  static uint8_t *smbios_entries;
 static size_t smbios_entries_len;
 static int smbios_type4_count = 0;
 
+static struct {
+    bool seen;
+    int headertype;
+    Location loc;
+} first_opt[2];
+
 static QemuOptsList qemu_smbios_opts = {
     .name = "smbios",
     .head = QTAILQ_HEAD_INITIALIZER(qemu_smbios_opts.head),
@@ -159,35 +165,20 @@  uint8_t *smbios_get_table(size_t *length)
  */
 static void smbios_check_collision(int type, int entry)
 {
-    uint16_t *num_entries = (uint16_t *)smbios_entries;
-    struct smbios_header *header;
-    char *p;
-    int i;
-
-    if (!num_entries)
-        return;
-
-    p = (char *)(num_entries + 1);
-
-    for (i = 0; i < *num_entries; i++) {
-        header = (struct smbios_header *)p;
-        if (entry == SMBIOS_TABLE_ENTRY && header->type == SMBIOS_FIELD_ENTRY) {
-            struct smbios_field *field = (void *)header;
-            if (type == field->type) {
-                error_report("SMBIOS type %d field already defined, "
-                             "cannot add table", type);
-                exit(1);
-            }
-        } else if (entry == SMBIOS_FIELD_ENTRY &&
-                   header->type == SMBIOS_TABLE_ENTRY) {
-            struct smbios_structure_header *table = (void *)(header + 1);
-            if (type == table->type) {
-                error_report("SMBIOS type %d table already defined, "
-                             "cannot add field", type);
+    if (type < ARRAY_SIZE(first_opt)) {
+        if (first_opt[type].seen) {
+            if (first_opt[type].headertype != entry) {
+                error_report("Can't mix file= and type= for same type");
+                loc_push_restore(&first_opt[type].loc);
+                error_report("This is the conflicting setting");
+                loc_pop(&first_opt[type].loc);
                 exit(1);
             }
+        } else {
+            first_opt[type].seen = true;
+            first_opt[type].headertype = entry;
+            loc_save(&first_opt[type].loc);
         }
-        p += le16_to_cpu(header->length);
     }
 }