Attention is currently required from: Julius Werner, Martin Roth, ron minnich.

Patch set 2:Code-Review -1

View Change

3 comments:

• File src/commonlib/include/commonlib/coreboot_tables.h:

  • Patch Set #2, 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:

  • Patch Set #2, 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.

  • Patch Set #2, Line 306: strlen(mainboard_part_number) + 1, LB_ENTRY_ALIGN);

    Isn't this (8->4) a functional change in all 7 instances?

To view, visit change 70158. To unsubscribe, or for help writing mail filters, visit settings.