fru: cleanup redux #88
Merged
Conversation
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
AlexanderAmelkin
suggested changes
Dec 29, 2018
pstrinkle
force-pushed
the
sasdfasdfasdf
branch
from
08836ac
to
b4691dd
Compare
|
PR cleaned up -- dropped extra changes from commits. |
pstrinkle
commented
Jan 2, 2019
pstrinkle
commented
Jan 2, 2019
pstrinkle
commented
Jan 2, 2019
pstrinkle
commented
Jan 2, 2019
pstrinkle
commented
Jan 2, 2019
pstrinkle
commented
Jan 2, 2019
|
Alexander: I went through the PR and noticed a few more issues I want to address, including a bad cherry-pick merge. I'll reply to this PR when it's worth your time to look at again. |
pstrinkle
force-pushed
the
sasdfasdfasdf
branch
from
b4691dd
to
74fffab
Compare
|
Alexander, I believe I have addressed the one outstanding comment from you, and the dozen or so new comments I made where I noticed unrelated lines being modified. This was a side effect of not perfect cherry-picking onto a new branch after the original patches were all based on a fairly substantial alignment cleanup. |
pstrinkle
force-pushed
the
sasdfasdfasdf
branch
from
fdfd5a2
to
0bb7d1b
Compare
|
Rebased the branch onto 0ca9c66 |
pstrinkle
commented
Jan 3, 2019
pstrinkle
force-pushed
the
sasdfasdfasdf
branch
from
0bb7d1b
to
c7b6c9f
Compare
|
Alexander, I removed the last unrelated change (the merging of two conditions). PTAL at your convenience. |
Fixup the following array bounds checking bugs: [lib/ipmi_fru.c:1003]: (style) Array index 'i' is used before limits check. [lib/ipmi_fru.c:1127]: (style) Array index 'i' is used before limits check. [lib/ipmi_fru.c:1262]: (style) Array index 'i' is used before limits check. Signed-off-by: Patrick Venture <venture@google.com>
Cleanup style in method ipmi_fru_oemkontron_get as well as add inverted logic checks to reduce indentation. Signed-off-by: Patrick Venture <venture@google.com>
Delete unused variable matchInstance. The variable is repeatedly assigned, but nothing reads it. Signed-off-by: Patrick Venture <venture@google.com>
Drop extraneous parentheses when returning a negative value. Signed-off-by: Patrick Venture <venture@google.com>
Use the macros defined in ipmi_cc for IPMI return codes instead of magic numbers. Signed-off-by: Patrick Venture <venture@google.com>
Cleanup ipmi_fru_upg_ekeying such that it exits from one place that handles cleanup. Signed-off-by: Patrick Venture <venture@google.com>
Add fru_cc_rq2big helper method to reduce duplicate code checking for specific size-based IPMI response codes. Signed-off-by: Patrick Venture <venture@google.com>
Mark ipmi_fru_query_new_value as static as it's only used internally in this object. Signed-off-by: Patrick Venture <venture@google.com>
Change ipmi_fru_query_new_value to return the bool type instead of an int that's being used as a boolean value. Signed-off-by: Patrick Venture <venture@google.com>
Convert ipmi_fru_oemkontron_edit to return a bool type instead of an int used as a bool. Signed-off-by: Patrick Venture <venture@google.com>
Fix ipmi_fru_picmg_ext_edit to use bools instead of an int treated as a boolean. Signed-off-by: Patrick Venture <venture@google.com>
Add two special return codes specific to the IPMI FRU commands. Signed-off-by: Patrick Venture <venture@google.com>
Use the return code macros instead of magic numbers. Signed-off-by: Patrick Venture <venture@google.com>
Use two macros defining the FRU block sizes instead of hard-coded magic values. Signed-off-by: Patrick Venture <venture@google.com>
Check against FRU_AREA_MAXIMUM_BLOCK_SZ instead of FRU_BLOCK_SZ when checking if the write chunk needs to be reduced. Apparently, that was the original intention, and then there was just a typo. In other places the same check is done properly. Signed-off-by: Patrick Venture <venture@google.com>
Swap calls to free() with calls to free_n() to leverage helper method and handle clearing pointers after freeing in one step. Signed-off-by: Patrick Venture <venture@google.com>
AlexanderAmelkin
approved these changes
Feb 13, 2019
|
Hurray! This was a long journey, but we did it. Thank you, Patrick for your efforts! I'm merging it now. |
Sign up for free
to subscribe to this conversation on GitHub.
Already have an account?
Sign in.
Add this suggestion to a batch that can be applied as a single commit.
This suggestion is invalid because no changes were made to the code.
Suggestions cannot be applied while the pull request is closed.
Suggestions cannot be applied while viewing a subset of changes.
Only one suggestion per line can be applied in a batch.
Add this suggestion to a batch that can be applied as a single commit.
Applying suggestions on deleted lines is not supported.
You must change the existing code in this line in order to create a valid suggestion.
Outdated suggestions cannot be applied.
This suggestion has been applied or marked resolved.
Suggestions cannot be applied from pending reviews.
Suggestions cannot be applied on multi-line comments.
Suggestions cannot be applied while the pull request is queued to merge.
Suggestion cannot be applied right now. Please check back later.
Fru cleanup. Clean branch, cherry-picked the changes where possible, recreated where not. Left out the static cleanup as well. That can be done more universally later.