Attention is currently required from: Angel Pons, Benjamin Doron, Kyösti Mälkki, Lean Sheng Tan, Maximilian Brune, Nico Huber, Sean Rhodes.
Patrick Rudolph has posted comments on this change. ( https://review.coreboot.org/c/coreboot/+/74121?usp=email )
Change subject: [RFC] drivers/option: Add option list in cbtables ......................................................................
Patch Set 14:
(6 comments)
File src/drivers/option/cfr.c:
https://review.coreboot.org/c/coreboot/+/74121/comment/43e1dae7_e3559354 : PS12, Line 36: cfr_str->data_length = strlen(string) + 1;
Do we need to limit the string length?
Why?
https://review.coreboot.org/c/coreboot/+/74121/comment/2e4685ee_3d39afd9 : PS12, Line 37: memcpy(cfr_str->data, string, cfr_str->data_length);
This assumes memory from current is initialized also for the padded bytes that we skip filling with […]
Done
https://review.coreboot.org/c/coreboot/+/74121/comment/9f771413_67c3db92 : PS12, Line 60: return 0;
What makes the helptext special to have this test?
Done
https://review.coreboot.org/c/coreboot/+/74121/comment/16df0b9c_bc781eb5 : PS12, Line 97: current += sm_write_ui_helptext(current, ui_helptext);
Hitting assert() with opt_name and ui_name being NULL seems a bit strong for the purpose, specially […]
Done
File src/drivers/option/cfr.c:
https://review.coreboot.org/c/coreboot/+/74121/comment/433d62e1_046ad753 : PS13, Line 230: if (sm_obj->ctor) {
If a dependency evaluates as 0 or does not resolve due to OPTFLAG_SUPPRESS, should this be evaluated […]
Removed special handling of OPTFLAG_SUPPRESS. Payload will take care of it.
https://review.coreboot.org/c/coreboot/+/74121/comment/11ab8bb3_bf3d6976 : PS13, Line 242: if (sm_obj->sm_enum.flags & CFR_OPTFLAG_SUPPRESS)
If an object is referenced as a dependency, is it allowed for it's constructor function to set the S […]
Dropped this code. The question remains what the payload should do when the dependency is SUPPRESSED, but the refereced variable reads as true.