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.

Sign up for GitHub

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

Jump to bottom

Minor InputStream-class fixes #48

Open
wants to merge 3 commits into
base: master
Choose a base branch
from
Open

Minor InputStream-class fixes #48

wants to merge 3 commits into from

Conversation

vifactor
Copy link

@vifactor vifactor commented Aug 12, 2024 •
edited
Loading

  • fix compiler warning for identical if-branches
In instantiation of 'bool CommonAPI::SomeIP::InputStream::_readBitValue(Type_&, uint8_t, bool) [with Type_ = unsigned char; uint8_t = unsigned char]':
[...]
CommonAPI/SomeIP/InputStream.hpp:633:17: warning: this condition has identical branches [-Wduplicated-branches]
  633 |                 if (isLittleEndian)
  • remove a related TODO statement by adding new class member
  • fix uninitialized variable

@vifactor
Fix -Wduplicated-branches compiler warning
bab5098 
When InputStream::_readBitValue template is instantiated
with sizeof(Type_)==1 an if-statement had two identical brances.
@vifactor
Make isLittleEndian flag a InputStream class member
2d6a57e 
hence remove TODO
@vifactor
Fix uninitialized variable in InputStream::readValue
16df239 
itsSize variable at certain execution paths could remain
uninitializedi (i.e. contain garbage) before assignment to maxSize

additionally, this change decreases the scope of maxSize

Signed-off-by: Viktor Kopp <[email protected]>
@vifactor
Copy link
Author

vifactor commented Nov 1, 2024

@fcmonteiro what would I need to do to have this considered? Thanks

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Reviewers
No reviews
Assignees
No one assigned
Labels
None yet
Projects
None yet
Milestone
No milestone
Development

Successfully merging this pull request may close these issues.
1 participant
@vifactor