Amelkin, Aleksandr
2014-05-20 08:05:56 UTC
Hi.
I've created a patch to address the ill-behaving FRU data decoding code. Please review.
Specifically, the patch does the following:
1. Fixes incorrect behavior on FRUs without Internal Use Area (which is allowed by the specification)
2. Corrects size calculation for Internal Use Area (finds the start of the actual next section instead of blindly assuming that the next section is Chasiss Info)
3. Reduces the amount of copy/pasted code and magic numbers
4. Corrects error messages to say the truth (that it couldn't read something) instead of spitting out false claims (that something was "invalid").
5. Reduces nesting level by getting rid of "while(0)/break" in favor of simple "goto".
The patch was originally created for 1.8.13, but has been adjusted to cleanly apply to 1.8.14.
I attached two versions of the patch. One (eka-1.8.14.diff) is the full version, and the other (eka-1.8.14-nospace.diff)
is the version containing essential changes only, omitting all whitespace changes (indentations, etc.) for ease of review.
It's a pity that 1.8.14 now doesn't allow calling 'ekanalyzer frushow sm=<filename>' without specifying
a full set of target options, for the aforementioned ekanalyzer command doesn't need them anyway and operates on a local file.
But that's another patch, I guess. :)
Another thing I would suggest to put on the ToDo list is adding checksum checking to this code.
With best regards -
Alexander Amelkin
I've created a patch to address the ill-behaving FRU data decoding code. Please review.
Specifically, the patch does the following:
1. Fixes incorrect behavior on FRUs without Internal Use Area (which is allowed by the specification)
2. Corrects size calculation for Internal Use Area (finds the start of the actual next section instead of blindly assuming that the next section is Chasiss Info)
3. Reduces the amount of copy/pasted code and magic numbers
4. Corrects error messages to say the truth (that it couldn't read something) instead of spitting out false claims (that something was "invalid").
5. Reduces nesting level by getting rid of "while(0)/break" in favor of simple "goto".
The patch was originally created for 1.8.13, but has been adjusted to cleanly apply to 1.8.14.
I attached two versions of the patch. One (eka-1.8.14.diff) is the full version, and the other (eka-1.8.14-nospace.diff)
is the version containing essential changes only, omitting all whitespace changes (indentations, etc.) for ease of review.
It's a pity that 1.8.14 now doesn't allow calling 'ekanalyzer frushow sm=<filename>' without specifying
a full set of target options, for the aforementioned ekanalyzer command doesn't need them anyway and operates on a local file.
But that's another patch, I guess. :)
Another thing I would suggest to put on the ToDo list is adding checksum checking to this code.
With best regards -
Alexander Amelkin