Nico Huber has posted comments on this change. ( https://review.coreboot.org/c/flashrom/+/33245 )
Change subject: cli: Add error on missing IFD ......................................................................
Patch Set 3:
(2 comments)
I am generally not opposed to strerror() like functions. But if we start writing those, we should probably unify the lib- flashrom return values, first? Maybe use a single enum and a single function returning error messages.
https://review.coreboot.org/#/c/33245/3/cli_classic.c File cli_classic.c:
https://review.coreboot.org/#/c/33245/3/cli_classic.c@613 PS3, Line 613: msg_gerr("Failed to read layout: %s\n", flashrom_layout_read_from_ifd_error_msg(ret)); With flashrom_layout_read_from_ifd() unaltered, it would now print the following on a read failure:
Reading ich descriptor... Read operation failed! FAILED! Failed to read layout: descriptor on flash couldn't be read
That seems a bit overkill.
https://review.coreboot.org/#/c/33245/3/libflashrom.c File libflashrom.c:
https://review.coreboot.org/#/c/33245/3/libflashrom.c@392 PS3, Line 392: const char *flashrom_layout_read_from_ifd_error_msg(const int err) It's generally a nice idea, but about the opposite how everything else in flashrom does it: Directly send the error message to the log.
I know the flashrom style is a bit hard to get into. But mixing different error-reporting mechanism might make it worse...