kbranigan / cJSON

I did not write this code, but I like it.
https://github.com/DaveGamble/cJSON
MIT License
394 stars 237 forks source link

cJSON_CreateNumber with unsigned number #38

Open ajaybhargav opened 7 years ago

ajaybhargav commented 7 years ago

cJSON_CreateNumber does not take care if number is unsigned. It always assume the input value is signed. There is no way to handle this. Maybe different function and different type identifier? e.g. cJSON_SNumber and cJSON_UNumber something...

FSMaxB commented 7 years ago

Internally all numbers are handled as double and only floating point numbers are supported in the JSON output anyways, so there is no point in adding additional functions for signed and unsigned integers.

According to RFC 7159, the internal representation of JSON numbers are implementation defined, but it's suggesting the use of 64 bit doubles.

cJSON represents numbers internally as double which means that the actual precision depends on the platform and C Compiler where it is compiled, but on most modern x86_64 systems it should be a 64 bit double as the RFC recommends.

Because numbers are stored as double in cJSONs internal data structures, providing additional functions for signed and unsigned integers doesn't make too much sense. You can just pass in signed and unsigned integers as is.

64 bit doubles can represent integers with up to 53 bit without any problems, if they get larger you will loose precision.

ajaybhargav commented 7 years ago

Let me explain why this requirement? cJSON provides following API: cJSON_CreateIntArray which takes int * as pointer, I cannot pass an unsigned char * to it even by typecasting it. Similar if I want to pass an unsigned int* I cannot do that. and as per the RFC document, there is no provision for long long or unsigned long long? maybe a representation as string array or something else?

FSMaxB commented 7 years ago

I wonder why these CreateArray functions exist in the first place, they are just convenience wrappers, you can achieve the same with a simple for loop:

mcJSON *CreateUintArray(unsigned int * uints, size_t array_size) {
    mcJSON *json_array = cJSON_CreateArray();

    for (size_t i = 0; i < array_size; i++) {
        mcJSON *number = cJSON_CreateNumber((double)uints[i]);
        cJSON_AddItemToArray(json_array, number);
    }

    return json_array;
}

Storing numbers as doubles is a good tradeoff for most users of cJSON I guess, so you would probably have to change it yourself.

But you can add an issue here, maybe DaveGamble will do something about. See https://github.com/DaveGamble/cJSON/issues/14 for reference.

ajaybhargav commented 7 years ago

I actually did that already... I thought maybe having such function in library itself would be helpful. Now I think its based on ones requirement if they need it or not. What about long long and unsigned long long? convert to string?

Another question: I see some difference in this repo and DaveGamble's repo. Which one is the latest?

FSMaxB commented 7 years ago

This repository didn't see any changes since Dave Gamble opened his repository. So Dave's is the latest.

ajaybhargav commented 7 years ago

OK thanks I will look into it.