Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Fixing logic error #16

Merged
merged 1 commit into from
May 31, 2022
Merged

Fixing logic error #16

merged 1 commit into from
May 31, 2022

Conversation

edtanous
Copy link
Contributor

Fix issue found by OpenBMC on latest version of gcc.

This file has quite a few cpp core guideline violations.  In this case,
it appears to be doing manual pointer arithmetic, without an end bound,
which is explicitly disallowed and ideally should be handled by rnage
checked.  This commit does not attempt to fix all the issues, but simply
fixes the compiler error such that the OpenBMC rebase can continue
without hitting this compiler error.

In this case, the check appears to be checking a pointer operation
against nullptr, which is somewhat nonsensical, as a pointer incremented
by an offset is nearly always going to be non nullptr.

What I suspect his code was attempting to do was to see if the end of
the string, passed by a null terminated character was equal to null, ie
'\0'.

This commit changes the logic to do as I suspect the author intended,
which will allow the build to succeed.

Tested: Code compiles in latest openbmc rebase.  No guard system
available to test functionality.

Signed-off-by: Ed Tanous <edtanous@google.com>
@RameshIyyar
Copy link
Collaborator

I agree @edtanous, I think the length should be a parameter to the constructor. I will check and change it. Thanks for pointing it out

Copy link
Collaborator

@devenrao devenrao left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

For now the fix is good, will create an issue to pass in the size rathe than just pointer.

@devenrao devenrao merged commit 380d99a into open-power:main May 31, 2022
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

3 participants