Attention is currently required from: Edward O'Callaghan, Light, Anastasia Klimchuk. Angel Pons has posted comments on this change. ( https://review.coreboot.org/c/flashrom/+/62764 )
Change subject: ich_descriptors.c: Ensure unsigned types >=0 on to prevent underflow ......................................................................
Patch Set 16:
(1 comment)
File ich_descriptors.c:
https://review.coreboot.org/c/flashrom/+/62764/comment/98f0a539_5620f3a8 PS14, Line 501: for (j = 0; j < (size_t)min(num_regions, 12); j++)
Not sure why you don't believe it is the max bounds in this case?
Because the second loop (the one which is modified in the patch) starts with 12, and runs until num_regions. So 12 is a starting point of the loop, not the maximum. We can't use "max" in the name.
I want to understand what exactly this 12 means.
It probably means that Intel initially thought that having 12 regions in the IFD would be more than enough for any future needs. Later, they realized that they needed more regions, so they added a new set of bitfields to have more regions. I imagine they preserved the field position for the original 12 regions for backwards compatibility purposes.
You can see the 12 here:
/* From Skylake on */ struct { uint32_t ext_read :4, ext_write :4, read :12, write :12; } mstr[MAX_NUM_MASTERS];