Nico Huber has posted comments on this change. ( https://review.coreboot.org/c/coreboot/+/35484 )
Change subject: util/supermicro: Add and use new tool smcbiosinfo ......................................................................
Patch Set 15: Code-Review+1
(5 comments)
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;
isn't printing a helpmessage doing it's job?
Yeah, right, I forgot how the code originally looked like. I'd say it's a success in case of 'h' but a failure in case of '?'.
https://review.coreboot.org/c/coreboot/+/35484/13/util/smc/smcbiosinfo/smcbi... File util/smc/smcbiosinfo/smcbiosinfo.c:
https://review.coreboot.org/c/coreboot/+/35484/13/util/smc/smcbiosinfo/smcbi... PS13, Line 113: ret = strtol(s, NULL, 0); No check if it could be parsed?
I think the easiest way is to provide `endptr` and check that `**endptr == '\0'` after the call. Oh, and check that `*s != '\0'`, i.e. non-empty string.
https://review.coreboot.org/c/coreboot/+/35484/15/util/supermicro/Makefile.i... File util/supermicro/Makefile.inc:
https://review.coreboot.org/c/coreboot/+/35484/15/util/supermicro/Makefile.i... PS15, Line 4: SMCBIOSINFOTOOL:= $(obj)/smcbiosinfo We also have an $(objutil) but, meh, I don't know when to use it...
https://review.coreboot.org/c/coreboot/+/35484/15/util/supermicro/Makefile.i... PS15, Line 6: util/supermicro/smcbiosinfo This could be $(dir), IIRC.
https://review.coreboot.org/c/coreboot/+/35484/15/util/supermicro/Makefile.i... PS15, Line 7: MAKE s/MAKE /HOSTCC/ ?