If a value is not valid during the DT or SYSINFO parsing,  we explicitly
set that to "Unknown Product" and "Unknown" for the product and
manufacturer respectively.  It's cleaner if we move the checks insisde
smbios_add_prop_si() and provide an alternative string in case the
primary is NULL or empty

pre-patch dmidecode
<snip>
Handle 0x0001, DMI type 1, 27 bytes
System Information
        Manufacturer: Unknown
        Product Name: Unknown Product
        Version: Not Specified
        Serial Number: Not Specified
        UUID: Not Settable
        Wake-up Type: Reserved
        SKU Number: Not Specified
        Family: Not Specified

[...]

post-patch dmidecode:

Handle 0x0001, DMI type 1, 27 bytes
System Information
        Manufacturer: Unknown
        Product Name: Unknown
        Version: Unknown
        Serial Number: Unknown
        UUID: Not Settable
        Wake-up Type: Reserved
        SKU Number: Unknown
        Family: Unknown
[...]

While at it make smbios_add_prop_si() add a string directly if the prop
node is NULL and replace smbios_add_string() calls with
smbios_add_prop_si(ctx, NULL, ....)

Signed-off-by: Ilias Apalodimas <ilias.apalodi...@linaro.org>
---
Changes since v2:
- refactor even more code and remove the smbios_add_string calls from the
  main code. Instead use smbios_add_prop() foir everything and add a
  default value, in case the parsed one is NULL or emtpy
Changes since v1:
- None

 lib/smbios.c | 73 +++++++++++++++++++++++++---------------------------
 1 file changed, 35 insertions(+), 38 deletions(-)

diff --git a/lib/smbios.c b/lib/smbios.c
index d7f4999e8b2a..444aa245a273 100644
--- a/lib/smbios.c
+++ b/lib/smbios.c
@@ -102,9 +102,6 @@ static int smbios_add_string(struct smbios_ctx *ctx, const 
char *str)
        int i = 1;
        char *p = ctx->eos;

-       if (!*str)
-               str = "Unknown";
-
        for (;;) {
                if (!*p) {
                        ctx->last_str = p;
@@ -134,11 +131,18 @@ static int smbios_add_string(struct smbios_ctx *ctx, 
const char *str)
  *
  * @ctx:       context for writing the tables
  * @prop:      property to write
+ * @dval:      Default value to use if the string is not found or is empty
  * Return:     0 if not found, else SMBIOS string number (1 or more)
  */
 static int smbios_add_prop_si(struct smbios_ctx *ctx, const char *prop,
-                             int sysinfo_id)
+                             int sysinfo_id, const char *dval)
 {
+       if (!dval || !*dval)
+               dval = "Unknown";
+
+       if (!prop)
+               return smbios_add_string(ctx, dval);
+
        if (sysinfo_id && ctx->dev) {
                char val[SMBIOS_STR_MAX];
                int ret;
@@ -151,8 +155,8 @@ static int smbios_add_prop_si(struct smbios_ctx *ctx, const 
char *prop,
                const char *str;

                str = ofnode_read_string(ctx->node, prop);
-               if (str)
-                       return smbios_add_string(ctx, str);
+
+               return smbios_add_string(ctx, str ? str : dval);
        }

        return 0;
@@ -161,12 +165,15 @@ static int smbios_add_prop_si(struct smbios_ctx *ctx, 
const char *prop,
 /**
  * smbios_add_prop() - Add a property from the devicetree
  *
- * @prop:      property to write
+ * @prop:      property to write. The default string will be written if
+ *             prop is NULL
+ * @dval:      Default value to use if the string is not found or is empty
  * Return:     0 if not found, else SMBIOS string number (1 or more)
  */
-static int smbios_add_prop(struct smbios_ctx *ctx, const char *prop)
+static int smbios_add_prop(struct smbios_ctx *ctx, const char *prop,
+                          const char *dval)
 {
-       return smbios_add_prop_si(ctx, prop, SYSINFO_ID_NONE);
+       return smbios_add_prop_si(ctx, prop, SYSINFO_ID_NONE, dval);
 }

 static void smbios_set_eos(struct smbios_ctx *ctx, char *eos)
@@ -228,11 +235,9 @@ static int smbios_write_type0(ulong *current, int handle,
        memset(t, 0, sizeof(struct smbios_type0));
        fill_smbios_header(t, SMBIOS_BIOS_INFORMATION, len, handle);
        smbios_set_eos(ctx, t->eos);
-       t->vendor = smbios_add_string(ctx, "U-Boot");
+       t->vendor = smbios_add_prop(ctx, NULL, "U-Boot");

-       t->bios_ver = smbios_add_prop(ctx, "version");
-       if (!t->bios_ver)
-               t->bios_ver = smbios_add_string(ctx, PLAIN_VERSION);
+       t->bios_ver = smbios_add_prop(ctx, "version", PLAIN_VERSION);
        if (t->bios_ver)
                gd->smbios_version = ctx->last_str;
        log_debug("smbios_version = %p: '%s'\n", gd->smbios_version,
@@ -241,7 +246,7 @@ static int smbios_write_type0(ulong *current, int handle,
        print_buffer((ulong)gd->smbios_version, gd->smbios_version,
                     1, strlen(gd->smbios_version) + 1, 0);
 #endif
-       t->bios_release_date = smbios_add_string(ctx, U_BOOT_DMI_DATE);
+       t->bios_release_date = smbios_add_prop(ctx, NULL, U_BOOT_DMI_DATE);
 #ifdef CONFIG_ROM_SIZE
        t->bios_rom_size = (CONFIG_ROM_SIZE / 65536) - 1;
 #endif
@@ -280,22 +285,19 @@ static int smbios_write_type1(ulong *current, int handle,
        memset(t, 0, sizeof(struct smbios_type1));
        fill_smbios_header(t, SMBIOS_SYSTEM_INFORMATION, len, handle);
        smbios_set_eos(ctx, t->eos);
-       t->manufacturer = smbios_add_prop(ctx, "manufacturer");
-       if (!t->manufacturer)
-               t->manufacturer = smbios_add_string(ctx, "Unknown");
-       t->product_name = smbios_add_prop(ctx, "product");
-       if (!t->product_name)
-               t->product_name = smbios_add_string(ctx, "Unknown Product");
+       t->manufacturer = smbios_add_prop(ctx, "manufacturer", "Unknown");
+       t->product_name = smbios_add_prop(ctx, "product", "Unknown");
        t->version = smbios_add_prop_si(ctx, "version",
-                                       SYSINFO_ID_SMBIOS_SYSTEM_VERSION);
+                                       SYSINFO_ID_SMBIOS_SYSTEM_VERSION,
+                                       "Unknown");
        if (serial_str) {
-               t->serial_number = smbios_add_string(ctx, serial_str);
+               t->serial_number = smbios_add_prop(ctx, NULL, serial_str);
                strncpy((char *)t->uuid, serial_str, sizeof(t->uuid));
        } else {
-               t->serial_number = smbios_add_prop(ctx, "serial");
+               t->serial_number = smbios_add_prop(ctx, "serial", "Unknown");
        }
-       t->sku_number = smbios_add_prop(ctx, "sku");
-       t->family = smbios_add_prop(ctx, "family");
+       t->sku_number = smbios_add_prop(ctx, "sku", "Unknown");
+       t->family = smbios_add_prop(ctx, "family", "Unknown");

        len = t->length + smbios_string_table_len(ctx);
        *current += len;
@@ -314,15 +316,12 @@ static int smbios_write_type2(ulong *current, int handle,
        memset(t, 0, sizeof(struct smbios_type2));
        fill_smbios_header(t, SMBIOS_BOARD_INFORMATION, len, handle);
        smbios_set_eos(ctx, t->eos);
-       t->manufacturer = smbios_add_prop(ctx, "manufacturer");
-       if (!t->manufacturer)
-               t->manufacturer = smbios_add_string(ctx, "Unknown");
-       t->product_name = smbios_add_prop(ctx, "product");
-       if (!t->product_name)
-               t->product_name = smbios_add_string(ctx, "Unknown Product");
+       t->manufacturer = smbios_add_prop(ctx, "manufacturer", "Unknown");
+       t->product_name = smbios_add_prop(ctx, "product", "Unknown");
        t->version = smbios_add_prop_si(ctx, "version",
-                                       SYSINFO_ID_SMBIOS_BASEBOARD_VERSION);
-       t->asset_tag_number = smbios_add_prop(ctx, "asset-tag");
+                                       SYSINFO_ID_SMBIOS_BASEBOARD_VERSION,
+                                       "Unknown");
+       t->asset_tag_number = smbios_add_prop(ctx, "asset-tag", "Unknown");
        t->feature_flags = SMBIOS_BOARD_FEATURE_HOSTING;
        t->board_type = SMBIOS_BOARD_MOTHERBOARD;

@@ -343,9 +342,7 @@ static int smbios_write_type3(ulong *current, int handle,
        memset(t, 0, sizeof(struct smbios_type3));
        fill_smbios_header(t, SMBIOS_SYSTEM_ENCLOSURE, len, handle);
        smbios_set_eos(ctx, t->eos);
-       t->manufacturer = smbios_add_prop(ctx, "manufacturer");
-       if (!t->manufacturer)
-               t->manufacturer = smbios_add_string(ctx, "Unknown");
+       t->manufacturer = smbios_add_prop(ctx, "manufacturer", "Unknown");
        t->chassis_type = SMBIOS_ENCLOSURE_DESKTOP;
        t->bootup_state = SMBIOS_STATE_SAFE;
        t->power_supply_state = SMBIOS_STATE_SAFE;
@@ -388,8 +385,8 @@ static void smbios_write_type4_dm(struct smbios_type4 *t,
 #endif

        t->processor_family = processor_family;
-       t->processor_manufacturer = smbios_add_string(ctx, vendor);
-       t->processor_version = smbios_add_string(ctx, name);
+       t->processor_manufacturer = smbios_add_prop(ctx, NULL, vendor);
+       t->processor_version = smbios_add_prop(ctx, NULL, name);
 }

 static int smbios_write_type4(ulong *current, int handle,
--
2.40.1

Reply via email to