Attention is currently required from: Jakub Czapiga, Jérémy Compostella, Nico Huber, Werner Zeh.
Julius Werner has posted comments on this change. ( https://review.coreboot.org/c/coreboot/+/79946?usp=email )
Change subject: [RFC] region: Turn region_end() into an inclusive region_last() ......................................................................
Patch Set 5:
(2 comments)
File src/commonlib/region.c:
https://review.coreboot.org/c/coreboot/+/79946/comment/6c284c6d_b1322ffa : PS5, Line 16: if (region_last(c) < region_offset(p)) What's the point of this change? All this is trying to check is that `c` doesn't wrap, so the equivalent of that should be `region_last(c) < region_offset(c)` (technically `region_last(c) + 1 < region_offset(c)` but the difference doesn't matter in this case so you can leave out the `+ 1`). I don't get why you'd want to check against `p` instead?
File tests/commonlib/region-test.c:
https://review.coreboot.org/c/coreboot/+/79946/comment/610a48a8_1f3d4e92 : PS5, Line 338: assert_true((uintptr_t)backing <= SIZE_MAX);
Ah, sorry, didn't mean to commit this. I ran into a problem here […]
I think a lot of our code assumes that coreboot is built and run on a flat linear address space, and that's fine. If we're worried about accidentally running on those kinds of host environments we should maybe rather assert that in some central place (e.g. have xcompile check that `SIZE_MAX` equals `UINTPTR_MAX` or something like that?).