Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

Already on GitHub? Sign in to your account

Big Integer (merging Deron's patch) #91

Closed
wants to merge 5 commits into
from

Conversation

Projects
None yet
3 participants

I've merged Deron's patch to the newest version of Jansson.

@akheron akheron commented on an outdated diff Oct 4, 2012

doc/apiref.rst
+
+.. function:: json_is_anynumber(const json_t *json)
+
+ Returns true for values of types ``JSON_INTEGER``,
+ ``JSON_BIGINTEGER``, ``JSON_REAL`` and ``JSON_BIGREAL``, and false
+ for other types and for *NULL*.
+
+.. function:: json_is_anyinteger(const json_t *json)
+
+ Returns true for values of types ``JSON_INTEGER`` and
+ ``JSON_BIGINTEGER``, and false for other types and for *NULL*.
+
+.. function:: json_is_anyreal(const json_t *json)
+
+ Returns true for values of types ``JSON_REAL`` and
+ ``JSON_BIGREAL``, and false for other types and for *NULL*.
.. function:: json_is_boolean(const json_t *json)
@akheron

akheron Oct 4, 2012

Owner

A blank line is missing here

@akheron

akheron Oct 4, 2012

Owner

Also, .. versionadded: 2.5 should be added to all new functions and macros, as this will be a part of the 2.5 release.

@akheron akheron and 1 other commented on an outdated diff Oct 4, 2012

doc/apiref.rst
@@ -851,6 +916,34 @@ is in UTF-8.
On error, the function should return -1 to stop the encoding
process. On success, it should return 0.
+ .. versionadded:: 2.4
@akheron

akheron Oct 4, 2012

Owner

This hunk is in the wrong place. It should be after the other JSON_* decoding flags.

@ripcurld0

ripcurld0 Oct 4, 2012

Piece of cake.
Done.

@akheron akheron and 1 other commented on an outdated diff Oct 4, 2012

doc/apiref.rst
Usually, you can safely use plain ``int`` in place of
``json_int_t``, and the implicit C integer conversion handles the
rest. Only when you know that you need the full 64-bit range, you
should use ``json_int_t`` explicitly.
+.. type:: double
+
+ The C type ``double`` is used to store JSON real values in the
+ absence of a big number extension.
+
+ Note that the C type ``long double`` is only supported by using the
+ big number extension API.
+
+.. type:: json_bigz_t
+.. type:: json_bigz_const_t
@akheron

akheron Oct 4, 2012

Owner

Are the json_bigz_const_t and json_bigr_const_t types really needed? I find them quite ugly. Would it work if the definition was just typedef JSON_BIGZ_TYPE json_bigz_t, so json_bigz_t (and json_bigr_t) weren't pointers? This way, the Jansson code would use json_bigz_t * and const json_bigz_t *.

@ripcurld0

ripcurld0 Oct 4, 2012

It will be painful. Hmmm...I think I can handle it, though.

@akheron

akheron Oct 5, 2012

Owner

I'd like to know whether it is possible and doesn't make the API harder to use. If so, then I think it should be changed.

@akheron akheron and 1 other commented on an outdated diff Oct 4, 2012

src/pack_unpack.c
+ case 'V':
@akheron

akheron Oct 4, 2012

Owner

IIRC, v and V unpack formats were supposed to be dropped from this patch.

@ripcurld0

ripcurld0 Oct 4, 2012

In my opinion, these two make it easier to work with big integers.
Eventually, you're the boss, thus, it's your call.

@akheron akheron and 1 other commented on an outdated diff Oct 4, 2012

doc/apiref.rst
``o`` (any value) [json_t \*]
Store a JSON value with no conversion to a :type:`json_t` pointer.
``O`` (any value) [json_t \*]
Like ``O``, but the JSON value's reference count is incremented.
+``v`` (any scalar) [json_t \*]
@akheron

akheron Oct 4, 2012

Owner

IIRC, v and V unpack formats were supposed to be dropped from this patch.

@ripcurld0

ripcurld0 Oct 5, 2012

I have taken off the v and V from the unpack formats. What about json_is_anynumber, json_is_anyinteger and json_is_anyreal? I think we also should take them off in order to keep on consistency in the code.

@akheron

akheron Oct 5, 2012

Owner

Why do you think they should be removed? I think they are useful when dealing with code that uses both bigints/bigreals and normal ints/reals.

@ripcurld0

ripcurld0 Oct 5, 2012

I am not sure, but it seems like if v and V unpack formats are out then there's no special purpose to these functions. I don't mind to keep them, though.

@akheron akheron and 1 other commented on an outdated diff Oct 4, 2012

doc/apiref.rst
@@ -1380,6 +1527,9 @@ behavior is needed.
Jansson's API functions to ensure that all memory operations use
the same functions.
+ Supplying *NULL* as the function pointer will restore the default
+ of using an internal :func:`memset()` based wrapper.
@akheron

akheron Oct 4, 2012

Owner

I don't understand the memset() reference. Is it a leftover from the memory_funcs thingie?

@ripcurld0

ripcurld0 Oct 4, 2012

Bloody hell, I didn't see that. My bad.
Done.

@rogerz rogerz and 1 other commented on an outdated diff Oct 6, 2012

doc/apiref.rst
``f`` (real) [double]
Convert a JSON real to C :type:`double`.
``F`` (integer or real) [double]
Convert a JSON number (integer or real) to C :type:`double`.
+``r`` (big real) [json_bigr_t]
+ Convert a JSON big real to a C :type:`json_bigr_t`. The returned
+ pointer will reference newly allocated big number; you are
+ responsible for eventually freeing it. You must have already
+ registered a suitable big number package extension.
+
+``R`` (any integer) [json_bigr_t]
@rogerz

rogerz Oct 6, 2012

Contributor

Should it be 'any real'?

@akheron

akheron Oct 6, 2012

Owner

Yes it should.

Okay, I think it's good to go. Did you find anything else?

Owner

akheron commented Jan 31, 2014

I've rebased Deron's patch on top of current master. See f8716ac.

@ripcurld0 ripcurld0 closed this Mar 24, 2014

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment