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

string_of_float and float_of_string are locale dependent #6701

Closed
vicuna opened this issue Dec 10, 2014 · 10 comments

Comments

Projects
None yet
2 participants
@vicuna
Copy link

commented Dec 10, 2014

Original bug ID: 6701
Reporter: @hhugo
Assigned to: @gasche
Status: resolved (set by @gasche on 2018-06-01T19:51:51Z)
Resolution: fixed
Priority: normal
Severity: minor
Target version: later
Fixed in version: 4.08.0+dev/beta1/beta2
Category: standard library
Related to: #6296
Monitored by: braibant @hhugo @gasche @ygrek Bardou @hcarty @dbuenzli

Bug description

archive from 2004, http://caml.inria.fr/pub/ml-archives/caml-list/2004/06/e8eba13a4e7977d4c49f83bb9ea0eff2.en.html

  • one need to document functions that rely on C code being platform/locale/.. dependent.
  • provide alternative functions not containing this issue/feature

Additional information

for example, Yojson will be unable to read/parse JSON containing floats.
ocaml-community/yojson#13

@vicuna

This comment has been minimized.

Copy link
Author

commented Jan 13, 2015

Comment author: @damiendoligez

If at all possible, we should make these functions locale-independent and (maybe) provide some explicitly locale-dependent functions.

@vicuna

This comment has been minimized.

Copy link
Author

commented Jun 8, 2015

Comment author: @hhugo

cross reference mirage/mirage-platform#118

@vicuna

This comment has been minimized.

Copy link
Author

commented Dec 5, 2015

Comment author: @xavierleroy

I'm postponing this PR to 'later', as there is no simple solution. For reference, here are some options:

  • Use strtod_l when available. But it's not widely available, e.g. it's not in Linux/Glibc.
  • Use setlocale() before strtod() to select the C locale, then use setlocale() again to restore the locale. Problem: probably expensive; not nice for multithreaded programs.
  • Use localeconv() to determine the current "radix character" (= decimal point), then rewrite the string before calling strtod() to replace "." by the radix character. Yuck.
  • Drop strtod() entirely and embark a portable reimplementation. Problem: that's an awful lot of code.
@vicuna

This comment has been minimized.

Copy link
Author

commented Dec 5, 2015

Comment author: @xavierleroy

Contradicting myself: strtod_l is indeed available in Linux/Glibc, it's just that it has no man page on my Linux installation. Perhaps it could be used, as a best effort.

@vicuna

This comment has been minimized.

Copy link
Author

commented May 9, 2017

Comment author: markghayden

Is there any progress on this? We have a German user on Ubuntu with issue because Yojson rejects valid JSON file because of locale issue converting strings to floats.

@vicuna

This comment has been minimized.

Copy link
Author

commented May 28, 2017

Comment author: @ygrek

strtod_l will work, but afaics in 2017 there is no obvious solution for linux to printf floats in locale-independent way..

Options:

  • vsnprintf_l seems only available in bsd, but not linux
  • uselocale() to change thread locale around all vsnprintf calls.. (or maybe around all entries into ocaml runtime?). It avoids threads race condition, but still feels heavy (we could maybe use Printf machinery to pass a flag if format contains float conversion?)
  • localeconv() decimal point rewriting, same yuck
@vicuna

This comment has been minimized.

Copy link
Author

commented May 28, 2017

Comment author: @ygrek

#1185

@vicuna

This comment has been minimized.

Copy link
Author

commented Jun 1, 2018

Comment author: @gasche

This should be fixed in trunk now that ygrek's updated PR #1803 was merged.

@vicuna vicuna closed this Jun 1, 2018

@vicuna

This comment has been minimized.

Copy link
Author

commented Jun 2, 2018

Comment author: @hhugo

Thanks to all of you for the fix.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
You can’t perform that action at this time.