Attention is currently required from: Aarya, Alexander Goncharov, Peter Marheine, Sam McNally.
Anastasia Klimchuk has posted comments on this change. ( https://review.coreboot.org/c/flashrom/+/77747?usp=email )
Change subject: erasure_layout: Fix double invocation of erasers ......................................................................
Patch Set 7:
(2 comments)
Patchset:
PS2:
I agree that the new condition seems unnecessary, since it duplicates the loop's termination conditi […]
I agree, removed that second invokation. Which means in comparison with head, this patch only removes code. Good way to fix a bug :)
While it is a bit scary to just remove the code, I do agree it is not needed, and given the earlier investigation I did (2 comments ago) it was most likely introduced by accident. The test at the end of this chain still passes.
Patchset:
PS3:
Thank you! I added one comment to the ticket. If you give me the images I can debug too! […]
Okay so I am deep into debugging https://ticket.coreboot.org/issues/494 and I think it's not related to this patch. It repros when there is an un-aligned layout region, and when region_end needs to be extended to align. Probably the case of extending region_start will hit the same bug too. But anyway, it seems like entirely different issue and I don't think we should block this patch for it.
Once I am done debugging, I will eventually sent a new patch for ticket 494.
Another point is that after fixing 494 I will add more test cases to the test which is now in CB:67535 . It would be great to submit the first version of the test, and then adding new test cases in further patches will be so much easier to review.
What do you think? is it alright to unblock this patch?