Attention is currently required from: Simon Buhrow, Nico Huber, Aarya, Anastasia Klimchuk.
Edward O'Callaghan has posted comments on this change. ( https://review.coreboot.org/c/flashrom/+/66104 )
Change subject: flashrom.c: Add wrapper function to use the erase algorithm ......................................................................
Patch Set 91:
(8 comments)
File erasure_layout.c:
https://review.coreboot.org/c/flashrom/+/66104/comment/8fb97c19_b1d7589d PS91, Line 1: /* you can use clang-format to help with formatting like line wraps or do it manually but whatever you do you should try to keep consistent with generally kernel C style and you wont be far off.
https://www.kernel.org/doc/html/v4.10/process/coding-style.html
https://review.coreboot.org/c/flashrom/+/66104/comment/45429ca9_5bcb04ee PS91, Line 165: s `const s`
https://review.coreboot.org/c/flashrom/+/66104/comment/e47d5bf7_b747cf71 PS91, Line 170: old_start_buf allocation never checked.
https://review.coreboot.org/c/flashrom/+/66104/comment/698e2517_e9b37ae5 PS91, Line 182: // add some `\n` to help readability of the function.
https://review.coreboot.org/c/flashrom/+/66104/comment/f1466463_521e44ef PS91, Line 188: ( trivial: ` (`
https://review.coreboot.org/c/flashrom/+/66104/comment/07bc1f69_04439fea PS91, Line 189: ( trivial: ` (`
https://review.coreboot.org/c/flashrom/+/66104/comment/37d0fadf_f54d1d10 PS91, Line 194: if(erase_layout[i].layout_list[j].selected) { consider identifying more base-cases to early return/continue/break to avoid deep loops and branches.
For example in this case, the inverse case gives you a continue base case: ``` if (!erase_layout[i].layout_list[j].selected) { continue; }
[..] ```
https://review.coreboot.org/c/flashrom/+/66104/comment/953096fc_fba6f404 PS91, Line 195: //execute erase : erasefunc_t *erasefn = lookup_erase_func_ptr(erase_layout[i].eraser); : ret = erasefn(flashctx, start_addr, block_len); : if (ret) { : msg_cerr("Failed to execute erase command for offset %#x to %#x.\n", start_addr, start_addr + block_len); : ret = -1; : goto _end; : } : //adjust curcontents : memset(curcontents+start_addr, erased_value, block_len); : //after erase make it unselected again : erase_layout[i].layout_list[j].selected = false; : msg_cdbg("E(%x:%x)", start_addr, start_addr + block_len - 1); : //verify erase : ret = check_erased_range(flashctx, start_addr, block_len); : if (ret) { : msg_cerr("Verifying flash. Erase failed for range %#x : %#x, Abort.\n", start_addr, start_addr + block_len - 1); : goto _end; : } integrate CB:70517