Reducing the size of the cJSON struct typedef about cjson HOT 9 CLOSED

cJSON_Type needs 9 bits, not 6:

1. cJSON_False
2. cJSON_True
3. cJSON_NULL
4. cJSON_Number
5. cJSON_String
6. cJSON_Array
7. cJSON_Object
8. cJSON_IsReference
9. cJSON_StringIsConst

That means it would fit in int16_t but not one byte.

There's a lot of potential for improvement, but I'm not sure what @DaveGamble imagines as the future of cJSON. If we could appoint a version number for the current version, a new version could be made, that does all the breaking changes.

from cjson.

Whups..

I missed the last two ones.... and obviously also miscounted.

from cjson.

I don't really have much of an imagination of the future of cJSON.
The design is naive. If I were writing it now, I'd not use n-trees or linked lists, and instead be a little more crafty with memory allocation.

On the topic of reducing the size of the struct... you could make a union for types, but you're only forestalling the inevitable, which is to require persistence of the original string, lock down direct access to the structs, use a much better memory layout and hence allow 1:1 parse-print, deferred numerical parsing and better CPU+memory usage.

Also, shortening the flags field won't do much if your CPU is aligning struct members. You might have an interesting chip that doesn't do this, but you'll probably pay a cost in misaligned loads for ptrs.

@FSMaxB - where do you see the project going?

from cjson.

What is important to me is that cJSON is currently used by a lot of people, so my goal is to make it as correct, bug free and secure as possible for all the users out there.

I don't think it makes too much sense to add more features to the library than it already has, only minor improvements to the status quo. Feature work should probably be put into one of the other JSON libraries out there.

from cjson.

That's a good argument for rejecting this type of request, and focussing on bug reports, right?

I have done some work towards improving the score for: https://github.com/miloyip/nativejson-benchmark. Perhaps we should set default parse settings to be more strict...? I've a few small changes to the numeric parser that improve things in some obscure edge cases too.

from cjson.

Yes, you're right. Let's focus on bugs. These small changes can be done by the users of the library on a case by case basis anyways.

What do you mean by being more strict? Accepting characters after the end of the JSON is a good feature in my opinion (if that is what you mean), and it can always be turned off by setting require_null_terminated.

The number parser is really awful because parsing floating point numbers correctly is quite a hard problem. I thought it might be possible to leave this to the C library.

from cjson.

These small changes can be done by the users of the library on a case by case basis anyways.

Agreed.

What do you mean by being more strict? Accepting characters after the end of the JSON is a good feature in my opinion (if that is what you mean), and it can always be turned off by setting require_null_terminated.

Well, I agree with you, but Milo Yip disagrees with both of us.

The number parser is really awful because parsing floating point numbers correctly is quite a hard problem. I thought it might be possible to leave this to the C library.

Yes, it's a deeply hard problem. I have some improvements to commit sometime that get us a few extra points on the nativejson benchmark with this. Leaving it to C library (atof/sscanf) can be problematic for embedded users, and doesn't necessarily do better than we do... But I'll investigate. There's a good argument there for a macro I think.

from cjson.

Well, I agree with you, but Milo Yip disagrees with both of us

But doesn't your PR in his repository fix this by setting require_null_terminated to 1?

from cjson.

Yes. But arguably our default should get the high-score. I'm not strongly inclined to argue it, but I can see that an argument could be made. I'd prefer to leave things as they are.

from cjson.