Attention is currently required from: Julius Werner. jacz@semihalf.com has posted comments on this change. ( https://review.coreboot.org/c/coreboot/+/48557 )
Change subject: tests: Add lib/fmap-test test case ......................................................................
Patch Set 5:
(10 comments)
File tests/lib/fmap-test.c:
https://review.coreboot.org/c/coreboot/+/48557/comment/fcadff6e_f1bafb6c PS4, Line 94: mem_rdev_fmap_ro.rdev.root = &mem_rdev_ro.rdev;
What are you trying to do here? This isn't legal. […]
Done
https://review.coreboot.org/c/coreboot/+/48557/comment/1be11c1c_31c3a0bf PS4, Line 97: rdev
...and then here you immediately overwrite it here again... […]
Done
https://review.coreboot.org/c/coreboot/+/48557/comment/db18d33c_c3ee34a6 PS4, Line 98: assert_int_equal(FMAP_SECTION_RO_VPD_START, rdev->region.offset);
Please use the region_device_offset() and region_device_sz() accessors instead of accessing rdev mem […]
Done
https://review.coreboot.org/c/coreboot/+/48557/comment/f888734e_38588355 PS4, Line 118: /* Function fmap_locate_area_as_rdev is not tested with NULL
See comment style I mentioned in the other patch (or on https://doc.coreboot.org/coding_style. […]
Done
https://review.coreboot.org/c/coreboot/+/48557/comment/bd2ebc5e_110e6a25 PS4, Line 154: */
How about actually writing to it here, and then maybe looking it up as an RO rdev again to confirm y […]
Done
https://review.coreboot.org/c/coreboot/+/48557/comment/93b833ff_b0c6fe7c PS4, Line 164: assert_int_equal(FMAP_SECTION_COREBOOT_START, ar.offset);
For regions we also have accessors (region_offset()/region_sz()).
Done
https://review.coreboot.org/c/coreboot/+/48557/comment/0e9d0763_bc883294 PS4, Line 204: ar.offset = 2142037;
Would be good to test both an out-of-bounds area (like you're doing here) and an incorrect in-bounds […]
Done
https://review.coreboot.org/c/coreboot/+/48557/comment/5bd75ce9_29d1f2e2 PS4, Line 221: fmap_locate_area("RW_SECTION_A", &ar);
Why is this call here? It does nothing and has nothing to do with the rest of the test. […]
Done
https://review.coreboot.org/c/coreboot/+/48557/comment/195f0589_f7eafd47 PS4, Line 251: fmap_locate_area(section_name, &ar);
same as above
Done
https://review.coreboot.org/c/coreboot/+/48557/comment/6fbd5bfd_819bc6a2 PS4, Line 261: war
typo
Done