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 >