lloyd / yajl

A fast streaming JSON parsing library in C.
http://lloyd.github.com/yajl
ISC License
2.15k stars 435 forks source link

yajl_gen_double uses a comma instead of a point #222

Open PaulCombal opened 4 years ago

PaulCombal commented 4 years ago

All is in the title. Here is sample code:

    yajl_gen_string_wrap(handle, RATE_STR);
    if (yajl_gen_double(handle, (double)achievement.global_achieved_rate) != yajl_gen_status_ok) {
            std::cerr << "failed to make json" << std::endl;
    }

Here is the output:

... "RATE":86,500000,"ICON":242 ...

So as you can see, this produces invalid JSON since the comma cannot be used here as a delimiter.

Is it normal, how can I solve this?

Thanks

Edit: my version: 2.1.0-2, my OS language setting is French if this can help, if it has to do with formatting from the os or anything

drjasonharrison-vp-eio commented 4 years ago

Looks like a localization issue. French using the comma where json would expect a dot.

PaulCombal commented 4 years ago

Indeed, with further testing I figured everything works like a charm using LC_NUMERIC=en_US.UTF-8 before launching my app. I don't think this is a feature. Will this be fixed by any chance? JSON is the same whatever the laguage.

drjasonharrison-vp-eio commented 4 years ago

Two options for fixing this on yajl side come to mind. The first is to call setlocale(LC_NUMERIC, "C") before the sprintf and use the returned value from setlocale to reset after the sprintf. This can also be done in the calling code of yajl.

The second option is for yajl to call localeconv and use the value of the returned struct lconv field decimal_point to determine if the locale should be changed, or if the decimal point in the output of the sprintf should have the decimal point be changed.

Since C locale is global to a program, it might be even better to avoid locale at all and check for comma in the output of sprintf in yajl_gen_double and replace it with a dot. This will have a negative performance impact.

5HT2 commented 2 years ago

Any updates on this?