Attention is currently required from: Andrey Petrov, Felix Singer, Jérémy Compostella, Martin L Roth, Nico Huber, Ronak Kanabar.
Felix Held has posted comments on this change by Nico Huber. ( https://review.coreboot.org/c/coreboot/+/79905?usp=email )
The change is no longer submittable: All-Comments-Resolved is unsatisfied now.
Change subject: region: Introduce region_create() functions ......................................................................
Patch Set 7:
(4 comments)
Patchset:
PS7: i like this change, but find that the currently rather implicit check of the return value being CB_SUCCESS or something else makes it more difficult to read compared to checking the region_create_untrusted return value against CB_SUCCESS
File src/include/cpu/x86/smm.h:
https://review.coreboot.org/c/coreboot/+/79905/comment/0cfb6685_c5bbcb6b?usp... : PS7, Line 147: if (region_create_untrusted(&r, (uintptr_t)ptr, len)) region_create_untrusted returns enum cb_err, so i'd prefer checking the return value against CB_SUCCESS. if i didn't mess up the logic here, that would be: if (region_create_untrusted(&r, (uintptr_t)ptr, len) != CB_SUCCESS)
File util/cbfstool/cbfstool.c:
https://review.coreboot.org/c/coreboot/+/79905/comment/1216d010_de3b6225?usp... : PS7, Line 334: if (region_create_untrusted( i'd prefer to check the return values for the two region_create_untrusted calls against CB_SUCCESS here too
File util/cbfstool/cse_serger.c:
https://review.coreboot.org/c/coreboot/+/79905/comment/8d007579_42e4d345?usp... : PS7, Line 833: if (region_create_untrusted(r, offset, size)) { same here