Attention is currently required from: Simon Buhrow, Edward O'Callaghan, Aarya.
Anastasia Klimchuk has posted comments on this change. ( https://review.coreboot.org/c/flashrom/+/65844 )
Change subject: flashrom.c: Add functions for new erase selction algorithm ......................................................................
Patch Set 83: Code-Review+1
(15 comments)
File erasure_layout.h:
https://review.coreboot.org/c/flashrom/+/65844/comment/0b2485f3_6208f8cf PS83, Line 37: int erase_write(struct flashctx *const flashctx, chipoff_t region_start, chipoff_t region_end, uint8_t* curcontents, uint8_t* newcontents, struct erase_layout *erase_layout, bool *all_skipped); We have a hard limit of 112 chars per line, this one is too long See dev guidelines: https://www.flashrom.org/Development_Guidelines#Coding_style
File erasure_layout.c:
https://review.coreboot.org/c/flashrom/+/65844/comment/c8829916_b88cf1d9 PS83, Line 203: ll->selected = need_erase(curcontents + start_addr, newcontents + start_addr, erase_len, flashctx->chip->gran, erased_value); This line also exceeds 112 chars. In general, I see some of the lines are too long, could you please check for all the code in this file?
https://review.coreboot.org/c/flashrom/+/65844/comment/1b1b4879_d4ca1422 PS83, Line 245: uint8_t* old_start_buf = (uint8_t*)malloc(old_start - region_start); new line before
https://review.coreboot.org/c/flashrom/+/65844/comment/9558e1a4_a11fffa4 PS83, Line 268: //s when you are going through code to check for line limit 112chars, try to add a space between `//` and the comment text
https://review.coreboot.org/c/flashrom/+/65844/comment/a466ee79_df8e2566 PS83, Line 268: //select erase functions new line before
https://review.coreboot.org/c/flashrom/+/65844/comment/154145ac_faf7c96d PS83, Line 277: for (i = 0; i < erasefn_count; i++) { new line before
https://review.coreboot.org/c/flashrom/+/65844/comment/414ca6fd_5aeda635 PS83, Line 285: comment is misaligned, too many tabs ?
https://review.coreboot.org/c/flashrom/+/65844/comment/8eca481d_9fd13e7b PS83, Line 325: //adjust curcontents new line before
https://review.coreboot.org/c/flashrom/+/65844/comment/051b7653_31a4f297 PS83, Line 336: *all_skipped = false; new line before
https://review.coreboot.org/c/flashrom/+/65844/comment/c5c3eb09_ff157075 PS83, Line 339: //write new line before
https://review.coreboot.org/c/flashrom/+/65844/comment/5a99ced4_373ff0f1 PS83, Line 343: trailing space (Gerrit shows it bright pink :))
https://review.coreboot.org/c/flashrom/+/65844/comment/a0f62bb2_99a82015 PS83, Line 352: //adjust curcontents new line before
https://review.coreboot.org/c/flashrom/+/65844/comment/2d45db1d_9a068488 PS83, Line 355: *all_skipped = false; new line before
https://review.coreboot.org/c/flashrom/+/65844/comment/bab7f13c_b86eec7b PS83, Line 366: free(old_start_buf); new line before
https://review.coreboot.org/c/flashrom/+/65844/comment/535f05ba_a632ce00 PS83, Line 368: msg_cinfo("Erase/write done from %x to %x\n", region_start, region_end); new line before