Name of user not set #1003143 has posted comments on this change. ( https://review.coreboot.org/c/coreboot/+/46458 )
Change subject: tests: Add lib/imd_cbmem-test test case ......................................................................
Patch Set 3:
(10 comments)
https://review.coreboot.org/c/coreboot/+/46458/3/tests/lib/Makefile.inc File tests/lib/Makefile.inc:
https://review.coreboot.org/c/coreboot/+/46458/3/tests/lib/Makefile.inc@24 PS3, Line 24: #
Please remove commented-out code unless you're uploading a work-in-progress commit.
Done
https://review.coreboot.org/c/coreboot/+/46458/3/tests/lib/imd_cbmem-test.c File tests/lib/imd_cbmem-test.c:
https://review.coreboot.org/c/coreboot/+/46458/3/tests/lib/imd_cbmem-test.c@... PS3, Line 5: * another header file
nit: missing trailing period
Done
https://review.coreboot.org/c/coreboot/+/46458/3/tests/lib/imd_cbmem-test.c@... PS3, Line 26:
please, no space after casts
Done
https://review.coreboot.org/c/coreboot/+/46458/3/tests/lib/imd_cbmem-test.c@... PS3, Line 28:
please, no space after casts
Done
https://review.coreboot.org/c/coreboot/+/46458/3/tests/lib/imd_cbmem-test.c@... PS3, Line 32: void cbmem_run_init_hooks(int is_recovery)
Maybe add a comment explaining the purpose of this no-op function?
Done
https://review.coreboot.org/c/coreboot/+/46458/3/tests/lib/imd_cbmem-test.c@... PS3, Line 34:
please, no space after casts
Done
https://review.coreboot.org/c/coreboot/+/46458/3/tests/lib/imd_cbmem-test.c@... PS3, Line 191: 0x10
I wonder... […]
IDs do not need to be sequential. Should I use some set of defines instead of these values in test functions?
https://review.coreboot.org/c/coreboot/+/46458/3/tests/lib/imd_cbmem-test.c@... PS3, Line 267: status
state?
Done
https://review.coreboot.org/c/coreboot/+/46458/3/tests/lib/imd_cbmem-test.c@... PS3, Line 269: (uintptr_t)
Is this cast required?
Yes. Compiler reports error if you make integer from void pointer without explicit cast.
https://review.coreboot.org/c/coreboot/+/46458/3/tests/lib/imd_cbmem-test.c@... PS3, Line 273: status
state?
Done