[PATCH 5/7] smbios: if a string value is unknown, use string number 0

[Ilias Apalodimas]

On Mon, 29 Jan 2024 at 23:05, Heinrich Schuchardt
<heinrich.schuchardt at canonical.com> wrote:
>
> The SMBIOS specification describes: "If a string field references no
> string, a null (0) is placed in that string field."
>
> Accordingly we should avoid writing a string "Unknown" to the SMBIOS table.
>
> dmidecode displays 'Not Specified' if the string number is 0.
>
> Commit 00a871d34e2f ("smbios: empty strings in smbios_add_string()")
> correctly identified that strings may not have length 0 as two
> consecutive NULs indentify the end of the string list. But the suggested
> solution did not match the intent of the SMBIOS specification.
>
> Fixes: 00a871d34e2f ("smbios: empty strings in smbios_add_string()")
> Signed-off-by: Heinrich Schuchardt <heinrich.schuchardt at canonical.com>
> ---
>  lib/smbios.c | 33 ++++++++++++++++++---------------
>  1 file changed, 18 insertions(+), 15 deletions(-)
>
> diff --git a/lib/smbios.c b/lib/smbios.c
> index 81908e89610..50072adb4e8 100644
> --- a/lib/smbios.c
> +++ b/lib/smbios.c
> @@ -135,13 +135,16 @@ static const struct map_sysinfo *convert_sysinfo_to_dt(const char *node, const c
>   *
>   * @ctx:       SMBIOS context
>   * @str:       string to add
> - * Return:     string number in the string area (1 or more)
> + * Return:     string number in the string area. 0 if str is NULL.
>   */
>  static int smbios_add_string(struct smbios_ctx *ctx, const char *str)
>  {
>         int i = 1;
>         char *p = ctx->eos;
>
> +       if (!str)
> +               return 0;
> +
>         for (;;) {
>                 if (!*p) {
>                         ctx->last_str = p;
> @@ -216,7 +219,7 @@ static int smbios_add_prop_si(struct smbios_ctx *ctx, const char *prop,
>         int ret;
>
>         if (!dval || !*dval)
> -               dval = "Unknown";
> +               dval = NULL;
>
>         if (!prop)
>                 return smbios_add_string(ctx, dval);
> @@ -378,19 +381,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", "Unknown");
> -       t->product_name = smbios_add_prop(ctx, "product", "Unknown");
> +       t->manufacturer = smbios_add_prop(ctx, "manufacturer", NULL);
> +       t->product_name = smbios_add_prop(ctx, "product", NULL);
>         t->version = smbios_add_prop_si(ctx, "version",
>                                         SYSINFO_ID_SMBIOS_SYSTEM_VERSION,
> -                                       "Unknown");
> +                                       NULL);
>         if (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", "Unknown");
> +               t->serial_number = smbios_add_prop(ctx, "serial", NULL);
>         }
> -       t->sku_number = smbios_add_prop(ctx, "sku", "Unknown");
> -       t->family = smbios_add_prop(ctx, "family", "Unknown");
> +       t->sku_number = smbios_add_prop(ctx, "sku", NULL);
> +       t->family = smbios_add_prop(ctx, "family", NULL);
>
>         len = t->length + smbios_string_table_len(ctx);
>         *current += len;
> @@ -409,12 +412,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", "Unknown");
> -       t->product_name = smbios_add_prop(ctx, "product", "Unknown");
> +       t->manufacturer = smbios_add_prop(ctx, "manufacturer", NULL);
> +       t->product_name = smbios_add_prop(ctx, "product", NULL);
>         t->version = smbios_add_prop_si(ctx, "version",
>                                         SYSINFO_ID_SMBIOS_BASEBOARD_VERSION,
> -                                       "Unknown");
> -       t->asset_tag_number = smbios_add_prop(ctx, "asset-tag", "Unknown");
> +                                       NULL);
> +       t->asset_tag_number = smbios_add_prop(ctx, "asset-tag", NULL);
>         t->feature_flags = SMBIOS_BOARD_FEATURE_HOSTING;
>         t->board_type = SMBIOS_BOARD_MOTHERBOARD;
>
> @@ -435,7 +438,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", "Unknown");
> +       t->manufacturer = smbios_add_prop(ctx, "manufacturer", NULL);
>         t->chassis_type = SMBIOS_ENCLOSURE_DESKTOP;
>         t->bootup_state = SMBIOS_STATE_SAFE;
>         t->power_supply_state = SMBIOS_STATE_SAFE;
> @@ -453,8 +456,8 @@ static void smbios_write_type4_dm(struct smbios_type4 *t,
>                                   struct smbios_ctx *ctx)
>  {
>         u16 processor_family = SMBIOS_PROCESSOR_FAMILY_UNKNOWN;
> -       const char *vendor = "Unknown";
> -       const char *name = "Unknown";
> +       const char *vendor = NULL;
> +       const char *name = NULL;
>
>  #ifdef CONFIG_CPU
>         char processor_name[49];
> --
> 2.43.0
>

Reviewed-by: Ilias Apalodimas <ilias.apalodimas at linaro.org>