patch to decode 64bit integers

[Valerio]

Hello,

I'm using jansson to decode/encode twitter json object. The Twitter
API uses 64bit integers, which are currently not supported by
jansson.

I produced a patch for jansson to parse 64bit ints, and tested it on
Linux (32 and 64). Just in case somebody might need the functionality,
here it is:

cd jansson-1.3
patch -p1 < jansson-1.3-64bit-int.patch

---
diff -Naur jansson-1.3/src/dump.c jansson-1.3.new/src/dump.c
--- jansson-1.3/src/dump.c 2010-06-12 13:44:15.000000000 -0600
+++ jansson-1.3.new/src/dump.c 2010-08-08 11:15:54.000000000 -0600
@@ -185,7 +185,7 @@
char buffer[MAX_INTEGER_STR_LENGTH];
int size;

- size = snprintf(buffer, MAX_INTEGER_STR_LENGTH, "%d",
json_integer_value(json));
+ size = snprintf(buffer, MAX_INTEGER_STR_LENGTH, "%lld",
json_integer_value(json));
if(size >= MAX_INTEGER_STR_LENGTH)
return -1;

diff -Naur jansson-1.3/src/jansson.h.in jansson-1.3.new/src/
jansson.h.in
--- jansson-1.3/src/jansson.h.in 2010-06-12 13:43:18.000000000 -0600
+++ jansson-1.3.new/src/jansson.h.in 2010-08-10 10:26:07.000000000
-0600
@@ -53,7 +53,7 @@
json_t *json_array(void);
json_t *json_string(const char *value);
json_t *json_string_nocheck(const char *value);
-json_t *json_integer(int value);
+json_t *json_integer(long long value);
json_t *json_real(double value);
json_t *json_true(void);
json_t *json_false(void);
@@ -140,13 +140,13 @@
}

const char *json_string_value(const json_t *string);
-int json_integer_value(const json_t *integer);
+long long json_integer_value(const json_t *integer);
double json_real_value(const json_t *real);
double json_number_value(const json_t *json);

int json_string_set(json_t *string, const char *value);
int json_string_set_nocheck(json_t *string, const char *value);
-int json_integer_set(json_t *integer, int value);
+int json_integer_set(json_t *integer, long long value);
int json_real_set(json_t *real, double value);


diff -Naur jansson-1.3/src/jansson_private.h jansson-1.3.new/src/
jansson_private.h
--- jansson-1.3/src/jansson_private.h 2010-06-12 13:43:18.000000000
-0600
+++ jansson-1.3.new/src/jansson_private.h 2010-08-08
18:54:04.000000000 -0600
@@ -41,7 +41,7 @@

typedef struct {
json_t json;
- int value;
+ long long value;
} json_integer_t;

#define json_to_object(json_) container_of(json_, json_object_t,
json)
diff -Naur jansson-1.3/src/load.c jansson-1.3.new/src/load.c
--- jansson-1.3/src/load.c 2010-06-10 11:49:16.000000000 -0600
+++ jansson-1.3.new/src/load.c 2010-08-08 11:16:34.000000000 -0600
@@ -52,7 +52,7 @@
int line, column;
union {
char *string;
- int integer;
+ long long integer;
double real;
} value;
} lex_t;
@@ -430,25 +430,25 @@
}

if(c != '.' && c != 'E' && c != 'e') {
- long value;
+ long long value;

lex_unget_unsave(lex, c);

saved_text = strbuffer_value(&lex->saved_text);
- value = strtol(saved_text, &end, 10);
+ value = strtoll(saved_text, &end, 10);
assert(end == saved_text + lex->saved_text.length);

- if((value == LONG_MAX && errno == ERANGE) || value > INT_MAX)
{
+ if(value == LLONG_MAX && errno == ERANGE) {
error_set(error, lex, "too big integer");
goto out;
}
- else if((value == LONG_MIN && errno == ERANGE) || value <
INT_MIN) {
+ else if(value == LLONG_MIN && errno == ERANGE) {
error_set(error, lex, "too big negative integer");
goto out;
}

lex->token = TOKEN_INTEGER;
- lex->value.integer = (int)value;
+ lex->value.integer = value;
return 0;
}

diff -Naur jansson-1.3/src/value.c jansson-1.3.new/src/value.c
--- jansson-1.3/src/value.c 2010-06-12 13:43:18.000000000 -0600
+++ jansson-1.3.new/src/value.c 2010-08-08 11:17:21.000000000 -0600
@@ -725,7 +725,7 @@

/*** integer ***/

-json_t *json_integer(int value)
+json_t *json_integer(long long value)
{
json_integer_t *integer = malloc(sizeof(json_integer_t));
if(!integer)
@@ -736,7 +736,7 @@
return &integer->json;
}

-int json_integer_value(const json_t *json)
+long long json_integer_value(const json_t *json)
{
if(!json_is_integer(json))
return 0;
@@ -744,7 +744,7 @@
return json_to_integer(json)->value;
}

-int json_integer_set(json_t *json, int value)
+int json_integer_set(json_t *json, long long value)
{
if(!json_is_integer(json))
return -1;
---

-Valerio

[Petri Lehtinen]

Valerio wrote:
> Hello,
>
> I'm using jansson to decode/encode twitter json object. The Twitter
> API uses 64bit integers, which are currently not supported by
> jansson.
>
> I produced a patch for jansson to parse 64bit ints, and tested it on
> Linux (32 and 64). Just in case somebody might need the functionality,
> here it is:

Thanks. As you might know, I have plans on changing the internal type
of json_integer to long. long long may have performance and
compatibility issues, so this patch cannot be applied to Jansson. (But
if I understood correctly, this wasn't even your intention.)

However, this shows me what needs to be done to change the internal
type of json_integer, I'll just need to replace long long with long,
and update the documentation.

Talking about 64-bit integers also made me think (again) about
supporting a bignum library, e.g. GMP. I might make a few experiments
with it at some point.

Petri

[Petri Lehtinen]

Valerio wrote:
> Hello,
>
> I'm using jansson to decode/encode twitter json object. The Twitter
> API uses 64bit integers, which are currently not supported by
> jansson.
>
> I produced a patch for jansson to parse 64bit ints, and tested it on
> Linux (32 and 64). Just in case somebody might need the functionality,
> here it is:
>
> cd jansson-1.3
> patch -p1 < jansson-1.3-64bit-int.patch

While looking at your patch a bit closer, I noticed that there are
some wrapped lines. Here's a fixed version:

---

+++ jansson-1.3.new/src/jansson_private.h 2010-08-0818:54:04.000000000 -0600

[Deron Meranda]

On Tue, Aug 10, 2010 at 2:11 PM, Petri Lehtinen <pe...@digip.org> wrote:
> Thanks. As you might know, I have plans on changing the internal type
> of json_integer to long. long long may have performance and
> compatibility issues, so this patch cannot be applied to Jansson. (But
> if I understood correctly, this wasn't even your intention.)
>
> However, this shows me what needs to be done to change the internal
> type of json_integer, I'll just need to replace long long with long,
> and update the documentation.

Note that on LLP64 platforms a "long" is just 32-bit while a
"long long" is 64. See
<http://www.unix.org/version2/whatsnew/lp64_wp.html>

Perhaps it may be best to abstract the types out to just the
header file; that way if anybody needs to change the
actual type for some use it would be fairly easy. At a
minimum, something like:

#ifdef _LONG_LONG
#define USE_LONG_LONG 1
#endif
#ifdef LONG_LONG_MAX
#define USE_LONG_LONG 1
#endif

#ifdef USE_LONG_LONG
typedef long long json_int;
#define JSON_INTFMT "lld"
#define JSON_STRTOINT strtoll
#else
typedef long json_int;
#define JSON_INTFMT "ld"
#define JSON_STRTOINT strtol
#endif

Of course for ISO C99 systems, there are already some
predefined macros and types which you should use
instead. So do:

#if __STDC_VERSION__ >= 199901L
#include <inttypes.h>
typedef intmax_t json_int;
#define JSON_INTFMT PRIdMAX
#define JSON_STRTOINT strtoimax
#else
...
#endif

See <http://www.opengroup.org/onlinepubs/9699919799/basedefs/inttypes.h.html>

--
Deron Meranda
http://deron.meranda.us/

[Valerio]

As Deron noticed, long is 32 bits on 32 bits architectures. I picked
long long to have common source between 32 and 64 bits and have a
quick solution.

I guess I dont'see the performance hit of long long? most modern CPU
have 128-bit memory read/write ...

[Deron Meranda]

On Tue, Aug 10, 2010 at 4:34 PM, Valerio <valerio...@gmail.com> wrote:
> As Deron noticed, long is 32 bits on 32 bits architectures. I picked
> long long to have common source between 32 and 64 bits and have a
> quick solution.
>
> I guess I dont'see the performance hit of long long? most modern CPU
> have 128-bit memory read/write ...

Rarely if ever is there a performance hit for using 'long long' rather
than 'long', though there could be a "mild" memory hit.

It's not true the other direction though, there can be significant
performance penalties for going too small. For instance using
short rather than int. This is why the ISO C99 standard defines
portable minimal-fast integer types (e.g., int_fast16_t), but not
portable maximal-fast types.

[Deron Meranda]

> Rarely if ever is there a performance hit for using 'long long' rather
> than 'long', though there could be a "mild" memory hit.

I should also mention the hopefully obvious, the difference in
possible performance among your integer storage size choices
will most likely be insignificant for any typical use of JSON data.
Those things mostly only matter for massive number crunching,
image processing, kernel coding, etc.

The most important things to consider are:

1. It's very portable

2. It uses the largest native integer size (practically) available
to prevent overflows.

[Petri Lehtinen]

You both made me convinced. After really thinking about it, my biggest
concern was not performance but the ease of use. I wanted that even if
long long is used, it's still possible to do this:

int a = json_integer_value(foo);

... and other things like this. It seems that at least gcc doesn't
warn about assigning to smaller integer types, even with all warnings
enabled.

So, I've changed things so that the configure script detects whether
long long is available or not. json_int_t is typedef'd to either `long
long' or `long' based on this test, and it is used as the underlying
type of json_integer. Conversions to and from string have also been
changes as Deron suggested.

I still need to update the documentation. When that's done, I'll push
the change to the "next" branch in github.

I still fear that for some people even long long isn't enough... :)

Petri

[Deron Meranda]

On Wed, Aug 11, 2010 at 2:49 PM, Petri Lehtinen <pe...@digip.org> wrote:
> I wanted that even if long long is used, it's still possible to do this:
>
>  int a = json_integer_value(foo);
>
> ... and other things like this. It seems that at least gcc doesn't
> warn about assigning to smaller integer types, even with all warnings
> enabled.

None of the various compilers I have access to will ever warn
about this, even is sizeof(int) < sizeof(long).

The ISO C99 standard states that if the "larger" numeric type's
value can be represented in the smaller type's range, it is
to be implicitly and silently converted keeping the same value.

However if the value can not be represented, then it is
implementation-defined as to what happens. You could
get a bogus value, a bit-anded portion, the MAX value,
or even a runtime signal. Most compilers seem to take
the bit-and approach, simply cutting off the most significant
bits whether data loss occurs or not.

Note that this means this is a runtime issue, not a
compile-time.

I guess all that jansson should do is:

1. Warn about this in the documentation; i.e., that
integer values may be larger than 'int'.

2. Make a type definition available, e.g., json_int_t

[Petri Lehtinen]

Deron Meranda wrote:
> On Wed, Aug 11, 2010 at 2:49 PM, Petri Lehtinen <pe...@digip.org> wrote:
> > I wanted that even if long long is used, it's still possible to do this:
> >
> >  int a = json_integer_value(foo);
> >
> > ... and other things like this. It seems that at least gcc doesn't
> > warn about assigning to smaller integer types, even with all warnings
> > enabled.
>
> None of the various compilers I have access to will ever warn
> about this, even is sizeof(int) < sizeof(long).
>

[snip]

> Note that this means this is a runtime issue, not a
> compile-time.
>
> I guess all that jansson should do is:
>
> 1. Warn about this in the documentation; i.e., that
> integer values may be larger than 'int'.
>
> 2. Make a type definition available, e.g., json_int_t

Yes, this is exactly what I'm going to do. Thanks for the input.