Attention is currently required from: Julius Werner, Martin Roth, ron minnich.
Peter Stuge has posted comments on this change. ( https://review.coreboot.org/c/coreboot/+/70158 )
Change subject: coreboot_tables: Make existing alignment conventions more explicit ......................................................................
Patch Set 2: Code-Review-1
(3 comments)
File src/commonlib/include/commonlib/coreboot_tables.h:
https://review.coreboot.org/c/coreboot/+/70158/comment/15765d41_bbf101f0 PS2, Line 110: #define LB_ENTRY_ALIGN 4 Should this come before lb_uint64_t on line 107 and be used there too?
File src/lib/coreboot_table.c:
https://review.coreboot.org/c/coreboot/+/70158/comment/b7e05c4e_5121cef3 PS2, Line 80: assert(IS_ALIGNED(rec->size, LB_ENTRY_ALIGN)); What happens if assert() fails?
Does any existing code in the same code path use assert() already?
I don't like that there can be a runtime failure, since that is likely a catastrophical error. Can it be avoided by doing something else?
Explicit serialization code would avoid it, but I do recognize that that is a larger and/or different change.
https://review.coreboot.org/c/coreboot/+/70158/comment/3bff3fa3_18d57c90 PS2, Line 306: strlen(mainboard_part_number) + 1, LB_ENTRY_ALIGN); Isn't this (8->4) a functional change in all 7 instances?