Attention is currently required from: Nico Huber.
1 comment:
File ich_descriptors.c:
if (content->ISL <= 16)
return CHIPSET_5_SERIES_IBEX_PEAK;
I see the same side effects and conclude that for these numbers
this change does not add assumptions.
You didn't factor in the addition of a new chipset with a large ISL value. That was the whole point.
I'm going to assume that what you call "the default case" is the
return statement on line 943 (after this change). In line 942, we
check `content->ISL != 16` always before the default case. So the
information you are missing is still there, it's just not in the
current state of the code inside an if statement.
The check you are referring to is for a warning that just tells us that the descriptor was not recognized. It does not enforce anything, unlike the original code which put Ibex Peak detection in an if-statement to enforce proper behavior when Ibex Peak is detected while making the intentions for default behavior obvious.
NB. The purpose of the code is to make flashrom work, not to provide
a manual for future development.
Code needs to be maintainable. If it's not obvious how to do future development with this code then that is a problem that we should definitely fix.
Unless there is an actual bug that is fixed with this patch we should revert it. The original code is more explicit in each of its codepaths which makes it easier for future developers to avoid problems.
To view, visit change 55651. To unsubscribe, or for help writing mail filters, visit settings.