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

Bugfix for various strtol/strtoll issues #1209

Closed
wants to merge 5 commits into from
Closed

Bugfix for various strtol/strtoll issues #1209

wants to merge 5 commits into from

Conversation

sbalko
Copy link
Contributor

@sbalko sbalko commented May 25, 2013

This fixes a number of issues in the existing strtoll/strtol (and variants) implementation. Notably, this fixes issues when trying to parse hex numbers (0x...) with an undefined (=0) base where strtoll/strtol will (according to spec) resort to the string representation to identify the base.

A number of test cases for various bases (2, 8, 10, 16) is included.

Generally, we experience these sort of issues in "corner cases" fairly often with the existing stdlib implementation in library.js. In light of asm.js being so fast, I wonder whether emscripten wouldn't be better of by replacing most of that with an existing C library which has matured over time?

@kripken
Copy link
Member

kripken commented May 28, 2013

I am hoping we will investigate porting an existing libc in the near future, yeah, definitely worth doing.

@kripken
Copy link
Member

kripken commented May 28, 2013

Can you please rebase to remove the merge commits (and preferably also the dummy commit)?

@sbalko
Copy link
Contributor Author

sbalko commented May 29, 2013

Done!

@kripken
Copy link
Member

kripken commented May 29, 2013

Thanks. I also meant to also get rid of the other dummy commits, but I'll do that locally.

@kripken
Copy link
Member

kripken commented May 29, 2013

Rebased and merged to incoming. Thanks!

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.

2 participants