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

Parsing of colors should not be locale dependent #46

Closed
simonschmeisser opened this issue Oct 24, 2018 · 2 comments · Fixed by #47
Closed

Parsing of colors should not be locale dependent #46

simonschmeisser opened this issue Oct 24, 2018 · 2 comments · Fixed by #47
Labels

Comments

@simonschmeisser
Copy link
Contributor

rgba.push_back(std::stof(pieces[i]));

uses std::stof which is locale dependent and will truncate floats to ints if the decimal seperator does not match

this is the same issue as addressed in #45 but not included there as neither I nor anybody else looked closely enough ...

@gavanderhoorn
Copy link

Would it be an idea to do an audit of the relevant packages for use of std::stof and similar methods and replace them where appropriate?

I believe you've (@simonschmeisser) have proposed a similar change (moving away from locale-affected methods) for MoveIt. Seeing as this could be a cross-package issue: perhaps refactor this all out into a separate package and use that everywhere?

@simonschmeisser
Copy link
Contributor Author

The change in MoveIt! is because boost::lexical_cast is inconsistent in itself (ie boost::lexical_cast<double>(boost::lexical_cast<std::string>(0.1)) will throw an exception)

but yes, it's going to be a recurrent theme as more packages discover the nice (and totally inconsistent) features in C++ 11 and want to move away from boost::lexical_cast (which is not locale dependent ...)

also note that the word "locale" does not appear a single time in boost::lexical_cast documentation so they might switch to using something broken in future releases ...

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
Projects
None yet
Development

Successfully merging a pull request may close this issue.

3 participants