Nico Huber has posted comments on this change. ( https://review.coreboot.org/c/coreboot/+/35484 )
Change subject: util/smc: Add and use new tool smcbiosinfo ......................................................................
Patch Set 9:
(2 comments)
I'm puzzled by the file in `vendorcode/`, isn't that usually code written by the vendor? Can't we just move everything into `util/smc/Makefile.inc`?
https://review.coreboot.org/c/coreboot/+/35484/9/util/smc/smcbiosinfo/smcbio... File util/smc/smcbiosinfo/smcbiosinfo.c:
https://review.coreboot.org/c/coreboot/+/35484/9/util/smc/smcbiosinfo/smcbio... PS9, Line 100: ret = 0; Usually it counts as failure, 0 should only be returned if the tool did it's job.
https://review.coreboot.org/c/coreboot/+/35484/9/util/smc/smcbiosinfo/smcbio... PS9, Line 164: memcpy(&sbu.boardid[1], CONFIG_SUPERMICRO_BOARDID, strlen(CONFIG_SUPERMICRO_BOARDID)); this can overflow, how about MIN(sizeof(sbu.boardid) - 1, ...)