Attention is currently required from: Andrey Petrov, Julius Werner, Jérémy Compostella, Martin L Roth, Nico Huber, Ronak Kanabar.
Maximilian Brune has posted comments on this change by Nico Huber. ( https://review.coreboot.org/c/coreboot/+/79905?usp=email )
Change subject: region: Introduce region_create() functions ......................................................................
Patch Set 7: Code-Review+2
(1 comment)
File src/commonlib/include/commonlib/region.h:
https://review.coreboot.org/c/coreboot/+/79905/comment/ee3e1ce1_f972d19d?usp... : PS1, Line 103: static inline int region_create_untrusted(
I think I am missing an actual example where the separation of the validity check and the region_c […]
By that logic all our decompressing functions would need to check where the destination buffer is pointing to and if that may be a bad location. Since apparently you cannot assume what the functions do with their input parameters. All API's make certain assumptions from the caller (that is part of the API design). You are just shifting the responsibility for checking if a region is valid to the region API. I think the checks should be made by the caller since the caller has much more context/knowledge and knows what it wants and what is valid or not. Although I admit in this case the check is rather obvious in a sense that if the check fails, it is obviously wrong no matter the caller. But I think this is all mostly a matter of style anyway.
But having said all that: I have dragged this patch on for long enough and despite what my text above may suggest, I don't have strong feelings about this. I am just happy someone actually puts in the effort. Sorry about he holdup.