flavio / qjson

QJson is a qt-based library that maps JSON data to QVariant objects.
http://qjson.sourceforge.net
GNU Lesser General Public License v2.1
288 stars 140 forks source link

Double parsing broken ? #28

Closed vrabeux closed 11 years ago

vrabeux commented 11 years ago

Hi,

I can't get the double parsing to work :

QByteArray array = "{ \"name\" : 0.9 }"; QJson::Parser parser; qDebug() << parser.parse(array);

Results in : QVariant(QVariantMap, QMap(("name", QVariant(double, 0) ) ) )

Expected : QVariant(QVariantMap, QMap(("name", QVariant(double, 0.9) ) ) )

Related unit tests are also failing (at least on my OS : Mac OS X)

[Tested in branches, master and 1_0_0] [Using clang++ and Qt5 (5.0.0)]

Am i missing something ? Can it be related to this commit 453ce33c156449ba6aa8629915501b1485efb07e ?

Thanks;

vrabeux commented 11 years ago

The following patch seems to fix the problem, but I'm having problems compiling json_scanner.cc after re-generating it with flex.

The patch :

git diff ../src/json_scanner.yy 
diff --git a/src/json_scanner.yy b/src/json_scanner.yy
index ca3bef0..5966f1c 100644
--- a/src/json_scanner.yy
+++ b/src/json_scanner.yy
@@ -102,7 +102,8 @@ null          {

 -?(([0-9])|([1-9][0-9]+))(\.[0-9]+)?([Ee][+\-]?[0-9]+)? {
                 m_yylloc->columns(yyleng);
-                *m_yylval = QVariant(strtod(yytext, NULL));
+               QByteArray barray = yytext;
+               *m_yylval = QVariant(barray.toDouble());
                 if (errno == ERANGE) {
                     qCritical() << "Number is out of range: " << yytext;
                     return yy::json_parser::token::INVALID;

And the compilation error :

json_scanner.cc:3846:21: error: out-of-line definition of 'LexerInput' does not match any declaration in 'yyFlexLexer' size_t yyFlexLexer::LexerInput( char* buf, size_t max_size ) ^~~~~~ json_scanner.cc:3873:19: error: out-of-line definition of 'LexerOutput' does not match any declaration in 'yyFlexLexer' void yyFlexLexer::LexerOutput( const char* buf, size_t size ) ^~~ 2 errors generated.

vrabeux commented 11 years ago

Still investigating .... It seems it is a locale problem 0.9 fails but 0,9 works .... with *m_yylval = QVariant(strtod(yytext, NULL));

Humm .... I have no idea on how to fix this ....

vrabeux commented 11 years ago

Ok, got it working : In order to have strod working with US decimal numbers, i need to change the locale :

include

setlocale(LC_NUMERIC,"en_US");

However this will affect all the program. Is there a a way to specify this just for JSON parsing ?

moio commented 11 years ago

At the moment, no, since lexer uses the standard C function. You found a bug - JSON defines the dot decimal as the separator, regardless of locale. For performance reasons, I would not use the Qt method, I will take a look to save and restore locale before and after lexing when I have the time.

Thanks!

flavio commented 11 years ago

We can solve the issue by using strtod_l which takes a locale_t instance as final parameter. This will allow the qjson code to perform the conversion using the 'C' locale (which uses . as digit separator) without affecting the locale of the whole application. I'll prepare a patch asap.

flavio commented 11 years ago

Fixed on master.

vrabeux commented 11 years ago

Hi,

Thanks a lot ! But the changes does not compile on Mac OS X :

qjson/src/json_scanner.h:62:9: error: unknown type name 'locale_t' locale_t m_C_locale;

flavio commented 11 years ago

Could you try to apply the following patch?

diff --git a/src/json_scanner.cpp b/src/json_scanner.cpp
index 7a4ef6c..79a3838 100644
--- a/src/json_scanner.cpp
+++ b/src/json_scanner.cpp
@@ -31,7 +31,6 @@

 #include <cassert>

-#include <locale.h>
 #include <xlocale.h>

 JSonScanner::JSonScanner(QIODevice* io)
diff --git a/src/json_scanner.h b/src/json_scanner.h
index fdcf0e7..9fc26d5 100644
--- a/src/json_scanner.h
+++ b/src/json_scanner.h
@@ -33,6 +33,8 @@

 #include "parser_p.h"

+#include <locale.h>
+
 namespace yy {
   class location;
   int yylex(YYSTYPE *yylval, yy::location *yylloc, QJson::ParserPrivate* driver);
vrabeux commented 11 years ago

Still no luck ... I m on OS X with clang.

flavio commented 11 years ago

@SilvioMoioli could you work on this issue on your mac? Mine is dead.

vrabeux commented 11 years ago

Yes of course ! I just created a pull request that seems to fix the problem. Sadly I could not use strtod_l .

flavio commented 11 years ago

Could you try with the following patch:

diff --git a/src/json_scanner.cpp b/src/json_scanner.cpp
index 7a4ef6c..41e73b8 100644
--- a/src/json_scanner.cpp
+++ b/src/json_scanner.cpp
@@ -31,8 +31,6 @@

 #include <cassert>

-#include <locale.h>
-#include <xlocale.h>

 JSonScanner::JSonScanner(QIODevice* io)
   : m_allowSpecialNumbers(false),
diff --git a/src/json_scanner.h b/src/json_scanner.h
index fdcf0e7..99b3f03 100644
--- a/src/json_scanner.h
+++ b/src/json_scanner.h
@@ -33,6 +33,9 @@

 #include "parser_p.h"

+#include <locale.h>
+#include <xlocale.h>
+
 namespace yy {
   class location;
   int yylex(YYSTYPE *yylval, yy::location *yylloc, QJson::ParserPrivate* driver);

BTW, strtod_l is definitely available on OSX...

vrabeux commented 11 years ago

Indeed, it is available (:-S my mistake). Anyway, this patch compiles and works great ! Thanks a lot !

flavio commented 11 years ago

I merged the patch on master.