Skip to content

Fix IPv4 address parsing due to not-so-portable scanf modifier #6524

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
Merged
merged 1 commit into from
Apr 6, 2018
Merged

Fix IPv4 address parsing due to not-so-portable scanf modifier #6524

merged 1 commit into from
Apr 6, 2018

Conversation

forGGe
Copy link
Contributor

@forGGe forGGe commented Apr 2, 2018

Description

If newlib is compiled without C99 switch, %hhu will be missing. Newlib sources:

https://github.com/eblot/newlib/blob/2a63fa0fd26ffb6603f69d9e369e944fe449c246/newlib/libc/stdio/vfscanf.c#L588-L596

Forum bug

Created as https://os.mbed.com/forum/bugs-suggestions/topic/30567/
Proceed there for more info.

Pull request type

[x] Fix
[ ] Refactor
[ ] New target
[ ] Feature
[ ] Breaking change

@forGGe
Fix IPv4 address parsing due to not-so-portable scanf modifier
bd47110 
Bug is raised when using newlib-based toolchains.
%hh format is only avaliable in scanf if newlib is compiled
with _WANT_IO_C99_FORMATS option.
@0xc0170 0xc0170 requested a review from kjbracey April 3, 2018 09:27
@0xc0170
Copy link
Contributor

0xc0170 commented Apr 3, 2018

Thanks @forGGe for the report and fix. Can you please sign https://os.mbed.com/contributor_agreement/ ?

@forGGe
Copy link
Contributor Author

forGGe commented Apr 3, 2018

Thanks @forGGe for the report and fix. Can you please sign https://os.mbed.com/contributor_agreement/ ?

Done.

https://os.mbed.com/users/forGGe/

kjbracey
kjbracey approved these changes Apr 3, 2018
View reviewed changes
Copy link
Contributor

@kjbracey kjbracey left a comment

Choose a reason for hiding this comment

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

I believe we are still sometimes using a newlib-nano that lacks C99 stuff. Very annoying - particularly due to lack of %llu for 64-bit.

Given that, this change is okay. Was going to nak it because I thought we were losing a 0-255 range check, but it turns out scanf doesn't actually range check anyway - it's always just been undefined behaviour if out of range.

@forGGe
Copy link
Contributor Author

forGGe commented Apr 3, 2018

Thanks for approving! Let me know if you need something here (I see jenkins is failing, dunno why).

@0xc0170
Copy link
Contributor

0xc0170 commented Apr 4, 2018

/morph build

@mbed-ci
Copy link

mbed-ci commented Apr 4, 2018

Build : SUCCESS

Build number : 1661
Build artifacts/logs : http://mbed-os.s3-website-eu-west-1.amazonaws.com/?prefix=builds/6524/

Triggering tests

/morph test
/morph uvisor-test
/morph export-build
/morph mbed2-build

@mbed-ci
Copy link

mbed-ci commented Apr 4, 2018

Exporter Build : SUCCESS

Build number : 1294
Build artifacts/logs : http://mbed-os.s3-website-eu-west-1.amazonaws.com/?prefix=builds/exporter/6524/

@mbed-ci
Copy link

mbed-ci commented Apr 4, 2018

Test : SUCCESS

Build number : 1454
Test logs :http://mbed-os-logs.s3-website-us-west-1.amazonaws.com/?prefix=logs/6524/1454

@0xc0170
Copy link
Contributor

0xc0170 commented Apr 5, 2018

/morph mbed2-build

@0xc0170 0xc0170 merged commit f5385e7 into ARMmbed:master Apr 6, 2018
@kjbracey kjbracey mentioned this pull request Jun 20, 2018
Closed
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Reviewers

@0xc0170 0xc0170 0xc0170 approved these changes

@kjbracey kjbracey kjbracey approved these changes

Assignees
No one assigned
Labels
release-version: 5.8.2
Projects
None yet
Milestone
No milestone
Development

Successfully merging this pull request may close these issues.
4 participants
@forGGe @0xc0170 @mbed-ci @kjbracey