[PATCH] smbios: Fix table when no string is present

[Matthias Brugger]

On 25/03/2021 01:38, Simon Glass wrote:
> Hi Matthias,
> 
> On Thu, 18 Mar 2021 at 00:30, <matthias.bgg at kernel.org> wrote:
>>
>> From: Matthias Brugger <mbrugger at suse.com>
>>
>> When no string is present in a table, next_ptr points to the same
>> location as eos. When calculating the string table length, we would only
>> reserve one \0. By spec a SMBIOS table has to end with two \0\0 when no
>> strings a present.
>>
>> Signed-off-by: Matthias Brugger <mbrugger at suse.com>
>>
>> ---
>>
>>  lib/smbios.c | 2 +-
>>  1 file changed, 1 insertion(+), 1 deletion(-)
> 
> There is a bug here but I don't think this is right fix. I remember
> worrying about this, making the same change as you did, reverting it
> and then forgetting about it :-( It has no tests.
> 
> You are effectively changing the definition of next_ptr here:
> 
>  * @next_ptr: pointer to the start of the next string to be added. When the
>  * table is not empty, this points to the byte after the \0 of the
>  * previous string.
> 

That's true.

> (there is a typo in that in the code)
> 
> I think that breaks adding new strings.
> 

Well it doesn't because when adding a new string, ctx->next_ptr gets overwritten
[1]. It is only used to calculate the lenght of the string in
smbios_string_table_len() to calculate the size of the table [2]. But yes it's
not the obvious way to handle the case when no string is present.

> Can you instead change smbios_string_table_len() to add 2?
> 

Do you mean returning 2 when no string is present (ctx->next_ptr == ctx->eos)?

Adding 2 will break the case when we have a string present, as a string already
finishes with '\0' and we only need a second '\0'.

Regards,
Matthias

[1] https://source.denx.de/u-boot/u-boot/-/blob/master/lib/smbios.c#L95
[2] https://source.denx.de/u-boot/u-boot/-/blob/master/lib/smbios.c#L196

>>
>> diff --git a/lib/smbios.c b/lib/smbios.c
>> index 7d463c84a9..d21d37cdac 100644
>> --- a/lib/smbios.c
>> +++ b/lib/smbios.c
>> @@ -153,7 +153,7 @@ static int smbios_add_prop(struct smbios_ctx *ctx, const char *prop)
>>  static void smbios_set_eos(struct smbios_ctx *ctx, char *eos)
>>  {
>>         ctx->eos = eos;
>> -       ctx->next_ptr = eos;
>> +       ctx->next_ptr = eos + 1;
>>         ctx->last_str = NULL;
>>  }
>>
>> --
>> 2.30.2
>>
> 
> Regards,
> Simon
>