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 float is local dependant #469

Open
gvollant opened this issue Jan 22, 2022 · 4 comments
Open

parsing float is local dependant #469

gvollant opened this issue Jan 22, 2022 · 4 comments

Comments

@gvollant
Copy link

In France, decimal separator is "," and not "."

strtod used in pugixml.cpp uses strtod which is local dependant

possible alternative:

  1. use own double parsing code like https://github.com/michael-hartmann/parsefloat
  2. use std::from_chars is c++17 is detected
@s-trinh
Copy link

s-trinh commented Jan 22, 2022

I think you can also define the locale in your code before calling pugixml, something like this to use . point decimal:

  if (std::setlocale(LC_ALL, "C") == NULL) {
    std::cerr << "Cannot set locale to C" << std::endl;
  }

See: https://en.cppreference.com/w/cpp/locale/setlocale

@gvollant
Copy link
Author

is std::setlocale always tread safe?

gvollant added a commit to gvollant/OpenXLSX that referenced this issue Jan 22, 2022
gvollant added a commit to gvollant/OpenXLSX that referenced this issue Jan 22, 2022
gvollant added a commit to gvollant/OpenXLSX that referenced this issue Jan 22, 2022
gvollant added a commit to gvollant/OpenXLSX that referenced this issue Jan 22, 2022
gvollant added a commit to gvollant/OpenXLSX that referenced this issue Jan 23, 2022
gvollant added a commit to gvollant/OpenXLSX that referenced this issue Jan 23, 2022
gvollant added a commit to gvollant/OpenXLSX that referenced this issue Jan 23, 2022
@zeux
Copy link
Owner

zeux commented Feb 5, 2022

The canonical recommendation is to use setlocale at the beginning of main, which automatically makes both parsing and converting to strings locale-independent. Unfortunately std::to_chars specifically doesn't enjoy wide support, so I don't think we should solve this problem with C++17, especially since this means changing behavior in a pretty significant way between standard versions. I'm not sure if there's a way to achieve the same result without calling setlocale in C.

@gvollant
Copy link
Author

gvollant commented Feb 5, 2022

Thank you for answering

I suggest using code like https://github.com/michael-hartmann/parsefloat (which can be converted to wide), which not use C or C++ standard library.

pugixml can be used on a multithreaded library or code which did not want change locale.

gvollant added a commit to gvollant/OpenXLSX that referenced this issue Mar 2, 2022
Fix pugixml float parsing local dependency
zeux/pugixml#469
troldal#133
using code from https://github.com/michael-hartmann/parsefloat
gvollant added a commit to gvollant/OpenXLSX that referenced this issue Mar 2, 2022
gvollant added a commit to gvollant/OpenXLSX that referenced this issue Mar 2, 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

No branches or pull requests

3 participants