On Thu, 18 Jan 2024 at 14:54, Heinrich Schuchardt <heinrich.schucha...@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.schucha...@canonical.com> > >> Reviewed-by: Simon Glass <s...@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.apalodi...@linaro.org>