Attention is currently required from: Felix Singer, Hsuan-ting Chen, Edward O'Callaghan, Nikolai Artemiev. Ricardo Quesada has posted comments on this change. ( https://review.coreboot.org/c/coreboot/+/59714 )
Change subject: util/cbfstool: Port elogtool to libflashrom ......................................................................
Patch Set 10:
(2 comments)
File util/cbfstool/uflashrom.c:
https://review.coreboot.org/c/coreboot/+/59714/comment/fc5b05d8_94477656 PS10, Line 108: flashrom_write nit: I'd add comment for flashrom_write() and flashrom_read() describing what they do and the return values. In particular whether it returns positive or negative values as error.
(see comment below)
https://review.coreboot.org/c/coreboot/+/59714/comment/e20a13a4_0297dbd3 PS10, Line 160: r |= nit: is this needed ?
context: flashrom_programmer_shutdown always return 0. But it seems that libflashrom() uses positives values for errors (e.g: flashrom_programmer_init).
so, if in the future flashrom_programmer_shutdown() gets updated and returns an error, it will be a positive value.
But at this point r could be -1. So, most probably the `r |= ` in `r |= flashrom_programmer_shutdown` is not needed (?)
-
Ricardo Quesada (Code Review)