Attention is currently required from: Thomas Heijligen, Angel Pons, Nikolai Artemiev.
Edward O'Callaghan has posted comments on this change. ( https://review.coreboot.org/c/flashrom/+/67723 )
Change subject: cli_classic.c: Early init of layout obscures invalid memory access ......................................................................
Patch Set 2:
(3 comments)
Commit Message:
https://review.coreboot.org/c/flashrom/+/67723/comment/fb30da1f_15337832 PS2, Line 7: cli_classic.c: Early init of layout obscures invalid memory access
Can you give the codepath for the invalid memory access and why the patch helps to fix this? […]
I agree with the second part of what you said. What happened was that `flashrom_layout_get_region_range()` was called on `layout := NULL` which resulted in a seg fault. This occurred when trying to use `--wp-region=` without a specified `--image=` from the cli. The previous patch in the chain addresses the actual crash and this patch is addressing the issue of how the compiler didn't warn to indicate the underlying issue.
Nik, can you follow up here for any further questions and take this over please. Also it would be good to add a unit-test to go with the previous patch in the chain to cover this precise case.
File cli_classic.c:
https://review.coreboot.org/c/flashrom/+/67723/comment/e37a4ba5_516ca56c PS2, Line 541: struct flashrom_layout *layout = NULL;
This means "no layout", which is true: initially, the flashrom classic CLI doesn't have any layout t […]
The issue is not that `NULL` means 'no layout' but rather it is the attempted use of a layout before perhaps there is one available. By early initialisation one obscures this compiler warning that a branch didn't explicitly handle the case 'no layout' before usage. Squelching a compiler warning that can indicate possibly issue with procedural control flow isn't a good idea which made the bug the previous commit in the chain go unnoticed.
https://review.coreboot.org/c/flashrom/+/67723/comment/9a8c90aa_6d9cc494 PS2, Line 1110: msg_gdbg("Valid layout could not be found without image.\n");
What does this message even mean?
Nik, can you come up with a better string here for your bug. These patches were made to get you started, please follow up with Angel here.