[PATCH v2 2/4] cmd: provide command to display SMBIOS information

[Ilias Apalodimas]

On Thu, 18 Jan 2024 at 14:54, Heinrich Schuchardt
<heinrich.schuchardt at canonical.com> wrote:
>
> On 1/18/24 13:39, Ilias Apalodimas wrote:
> > Hi Heinrich,
> >
> > A few nits below
> >
> > On Wed, Jan 17, 2024 at 04:33:45PM +0100, Heinrich Schuchardt wrote:
> >> U-Boot can either generated an SMBIOS table or copy it from a prior boot
> >> stage, e.g. QEMU.
> >>
> >> Provide a command to display the SMBIOS information.
> >>
> >> Currently only type 1 and 2 are translated to human readable text.
> >> Other types may be added later. Currently only a hexdump and the list of
> >> strings is provided for these.
> >>
> >> Signed-off-by: Heinrich Schuchardt <heinrich.schuchardt at canonical.com>
> >> Reviewed-by: Simon Glass <sjg at chromium.org>
> >> ---
> >> v2:
> >
> > [...]
> >
> >>      email address updated
> >> +static struct smbios_header *next_table(struct smbios_header *table)
> >> +{
> >> +    const char *str;
> >> +
> >> +    if (table->type == SMBIOS_END_OF_TABLE)
> >> +            return NULL;
> >> +
> >> +    str = smbios_get_string(table, 0);
> >> +    return (struct smbios_header *)(++str);
> >
> > That can lead to unaligned access when we dereference that pointer, do we
> > care?
>
> SMBIOS tables are always byte aligned. This is why struct smbios_header
> is marked as packed. The GCCj documentation has this sentence:
>
> "The packed attribute specifies that a variable or structure field
> should have the smallest possible alignment - one byte for a variable,
> and one bit for a field, unless you specify a larger value with the
> aligned attribute."
>
> So shouldn't the compiler care about non-alignment? If there were a
> problematic usage, GCC would throw -Waddress-of-packed-member.

I don't think we have the strict alignment enabled in our makefiles.
But, that won't be a problem, I missed the packed attribute in the
smbios header.

with or without the change in smbios_print_generic()
Reviewed-by: Ilias Apalodimas <ilias.apalodimas at linaro.org>