Attention is currently required from: Arthur Heymans.
Julius Werner has posted comments on this change by Arthur Heymans. ( https://review.coreboot.org/c/coreboot/+/84140?usp=email )
Change subject: util/cbfstool: Make sure to only compare PT_LOAD stages ......................................................................
Patch Set 2:
(2 comments)
File util/cbfstool/cbfs-mkstage.c:
https://review.coreboot.org/c/coreboot/+/84140/comment/cdf20f6d_99fcfdf9?usp... : PS2, Line 295: xip_ This function is currently only used for XIP parsing but I don't think it fundamentally only works for XIP stages?
https://review.coreboot.org/c/coreboot/+/84140/comment/af791ea6_5cde26d6?usp... : PS2, Line 317: if (!prev) What you do here works, but honestly I had to re-read it 3 times to understand what's going on. Updating prev at the top of the loop and relying on the fact that cur is not yet updated is not at all intuitive.
How about, as an alternative, you rewrite the loop from here to say ``` if (prev && (prev->p_paddr + ... || ...)) { ...ERROR... }
prev = cur; } ```
I feel like that would be a much more normal way to write the "keep track of previous pointer" pattern, and achieves the same goal of not counting the non-PT_LOAD entries.