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.
Best regards
Heinrich
+}
+
+static void smbios_print_generic(struct smbios_header *table)
+{
+ char *str = (char *)table + table->length;
+
Do we want the header below printed if there are no strings?
IOW can we exit early if (!*str) ?
+ if (CONFIG_IS_ENABLED(HEXDUMP)) {
+ printf("Header and Data:\n");
+ print_hex_dump("\t", DUMP_PREFIX_OFFSET, 16, 1,
+ table, table->length, false);
+ }
+ if (*str) {
+ printf("Strings:\n");
+ for (int index = 1; *str; ++index) {
+ printf("\tString %u: %s\n", index, str);
+ str += strlen(str) + 1;
+ }
+ }
+}
+
+void smbios_print_str(const char *label, void *table, u8 index)
+{
+ printf("\t%s: %s\n", label, smbios_get_string(table, index));
+}
+
Other than that, LGTM
Thanks
/Ilias
