Hi Simon,
On Thu, 30 Nov 2023 at 04:46, Simon Glass <s...@chromium.org> wrote: > Hi Ilias, > > On Mon, 27 Nov 2023 at 10:11, 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_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.apalodi...@linaro.org> > > Reviewed-by: Peter Robinson <pbrobin...@gmail.com> > > Tested-by: Peter Robinson <pbrobin...@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