On Mon, 29 Jan 2024 at 23:05, Heinrich Schuchardt
<heinrich.schucha...@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.schucha...@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.apalodi...@linaro.org>
