Attention is currently required from: Furquan Shaikh, Ricardo Quesada. Jack Rosenthal has posted comments on this change. ( https://review.coreboot.org/c/coreboot/+/56883 )
Change subject: elogtool: add "clear" command ......................................................................
Patch Set 2:
(13 comments)
File src/commonlib/bsd/elog.c:
https://review.coreboot.org/c/coreboot/+/56883/comment/b768d3f6_427fe630 PS2, Line 50: mybin2bcd I think you can just move the function to common bsd code instead of worrying about gpl implications for a 1 line helper function
https://review.coreboot.org/c/coreboot/+/56883/comment/594c5f8b_cffd17b4 PS2, Line 57: indentation nit: since there's stuff on the line above beyond the (, you need to indent to the (
https://review.coreboot.org/c/coreboot/+/56883/comment/4f9cda34_814a36dd PS2, Line 66: sanity drop "sanity" (inclusive language)
File src/commonlib/bsd/include/commonlib/bsd/elog.h:
https://review.coreboot.org/c/coreboot/+/56883/comment/81ebda4c_7751ef4d PS2, Line 317: same indentation comment as above
File src/drivers/elog/elog.c:
https://review.coreboot.org/c/coreboot/+/56883/comment/cd24fbd4_5fd83aff PS2, Line 793: 0 spaces inside of braces
File util/cbfstool/elogtool.c:
https://review.coreboot.org/c/coreboot/+/56883/comment/b0c7a2bb_afddacbb PS2, Line 134: + 1 using <= you can drop the +1
https://review.coreboot.org/c/coreboot/+/56883/comment/aa0ca413_d9bfb3e4 PS2, Line 144: (const char *)(event) is the cast necessary?
https://review.coreboot.org/c/coreboot/+/56883/comment/87cda2e7_cfbd53e6 PS2, Line 155: indentation
https://review.coreboot.org/c/coreboot/+/56883/comment/af8c9887_69250e1e PS2, Line 166: NULL no input file option for clear command?
https://review.coreboot.org/c/coreboot/+/56883/comment/bc4d860c_da9f3c96 PS2, Line 176: "RW_ELOG" since we've used this constant twice now, would be nice to make a #define for it
https://review.coreboot.org/c/coreboot/+/56883/comment/2cc4b1af_13266bf3 PS2, Line 177: (void *) nit: api expects a uint8_t *
File util/cbfstool/eventlog.c:
https://review.coreboot.org/c/coreboot/+/56883/comment/092119df_96629635 PS2, Line 654: void lhs is void *, rhs is uint32_t * ... perhaps just type the variable as a uint32_t *
https://review.coreboot.org/c/coreboot/+/56883/comment/7d22cc48_961ab28b PS2, Line 654: drop space after cast