Hi Simon,
On Thu, 30 Nov 2023 at 04:46, Simon Glass sjg@chromium.org wrote:
Hi Ilias,
On Mon, 27 Nov 2023 at 10:11, Ilias Apalodimas ilias.apalodimas@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_string() and always report "Unknown" regardless of the missing field.
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 [...]
Signed-off-by: Ilias Apalodimas ilias.apalodimas@linaro.org Reviewed-by: Peter Robinson pbrobinson@gmail.com Tested-by: Peter Robinson pbrobinson@gmail.com
Changes since v1:
- None
lib/smbios.c | 17 +++-------------- 1 file changed, 3 insertions(+), 14 deletions(-)
diff --git a/lib/smbios.c b/lib/smbios.c index d7f4999e8b2a..fcc8686993ef 100644 --- a/lib/smbios.c +++ b/lib/smbios.c @@ -102,7 +102,7 @@ static int smbios_add_string(struct smbios_ctx *ctx,
const char *str)
I suggest that this function have an additional str property indicating the default string. Then you can pass DEFAULT_VAL or something like that, to each call.
Why? Do you think we need to fill in something different that "unknown"?
int i = 1; char *p = ctx->eos;
if (!*str)
if (!str || !*str)Does it ever happen that the string is present but empty?
With the changes on this patchset yes.
str = "Unknown"; for (;;) {@@ -151,8 +151,7 @@ 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); } return 0;@@ -231,7 +230,7 @@ static int smbios_write_type0(ulong *current, int
handle,
t->vendor = smbios_add_string(ctx, "U-Boot"); t->bios_ver = smbios_add_prop(ctx, "version");
if (!t->bios_ver)
if (!strcmp(ctx->last_str, "Unknown"))That is really ugly...checking the ctx value looking for a side effect.
Can you not have smbios_add_prop() continue to return NULL in this case?
Hmm I don't know, but I wonder why I am not just checking t->bios_ver for Unknown. I'll have a look and change it
[...]
Thanks /Ilias