Skip to content
New issue

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

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Consider changing name of "bool" type in cJSON.c introduced in v1.0.1 #71

Closed
kevinmkane opened this issue Nov 24, 2016 · 8 comments
Closed

Comments

@kevinmkane
Copy link

If cJSON.c ends up being compiled with a C++ compiler, it complains because as of commit 6790049 cJSON.c internally defines a bool type to be int. Commenting it out then can lead to problems because the header file still has the parameters as 'int'.

Consider introducing a type with a different name, and using it uniformly in both header file and implementation.

@FSMaxB
Copy link
Collaborator

FSMaxB commented Nov 24, 2016

I will use a precompiler macro instead.

I can introduce the new type in v2.0.0.

@FSMaxB
Copy link
Collaborator

FSMaxB commented Nov 24, 2016

I take that back. You shouldn't be compiling cJSON with a C++ compiler. It is compatible with C89. It doesn't claim to be compatible with C89 and C++ and at least currently there is no plan to make it work with both.

Use a C compiler to compile cJSON and then you can link the result with your C++ application.

@FSMaxB
Copy link
Collaborator

FSMaxB commented Nov 24, 2016

I will close this issue as wontfix.

Conforming to both C89 and C++98 is probably quite an effort and is not a goal of this library at present.

Note that there are quite a lot of good C++ JSON libraries out there, and you can still link to objects or shared library versions of cJSON compiled with a C compiler.

@FSMaxB FSMaxB closed this as completed Nov 24, 2016
@FSMaxB FSMaxB added the wontfix label Nov 24, 2016
@kevinmkane
Copy link
Author

I'll ask about changing our build system. It only occurred on our CI build for Arduino's cross-compiler for our project.

For what it's worth, this bool type was the only thing that caused a build break with the C++ compiler. When I reverted just that one commit, everything else builds fine. So please do consider taking this change since it's small.

@rzr
Copy link

rzr commented Nov 24, 2016

Let me cross link this issue to iotivity project,
https://gerrit.iotivity.org/gerrit/#/c/14625/3/extlibs/cjson/README-cJSON.txt

we'd like to avoid patching cJSON and be aligned to next upstream release if planned.

FSMaxB added a commit that referenced this issue Nov 25, 2016
@FSMaxB
Copy link
Collaborator

FSMaxB commented Nov 25, 2016

I renamed the boolean type to cjbool. So it works for now. Keep in mind though, that building with a C++ compiler can break at any time in the future.

If you would like cJSON to be genuinely compatible with C++ compilers, you can open a new issue and provide information and resources on how to achieve and maintain this, then I might reconsider, depending on the effort. I know that Lua is written in a C that is fully compatible with C++ but I didn't find much information on that. And most information I found is related to C99, not C89.

@FSMaxB FSMaxB removed the wontfix label Nov 25, 2016
@kevinmkane
Copy link
Author

Great, thanks!

@rzr
Copy link

rzr commented Nov 25, 2016

nice

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

No branches or pull requests

3 participants