On Thu, Dec 7, 2023 at 10:17 PM Ilias Apalodimas
<ilias.apalodi...@linaro.org> wrote:
>
> 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>
Reviewed-by: Peter Robinson <pbrobin...@gmail.com>
Tested-by: Peter Robinson <pbrobin...@gmail.com>
> ---
> 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
>
