Skip to content

CJSON_AS4CPP incompatable with original cJSON #1829

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

Open
scottpakula opened this issue Dec 22, 2021 · 15 comments
Open

CJSON_AS4CPP incompatable with original cJSON #1829

scottpakula opened this issue Dec 22, 2021 · 15 comments
Labels
bug This issue is a bug. Cmake Cmake related submissions p2 This is a standard priority issue

Comments

@scottpakula
Copy link

I had recently upgraded from aws-sdk-cpp 1.9.96 to 1.9.160, and in this update, the cJSON.h has been customized to redefine many functions to use the CJSON_AS4CPP convention.

The previous implementation looks like this:
https://github.com/aws/aws-sdk-cpp/blob/b7ad1b3e405ec730f1247f049bd700e7a3666622/aws-cpp-sdk-core/include/aws/core/external/cjson/cJSON.h

The current implementation looks like this:
https://github.com/aws/aws-sdk-cpp/blob/main/aws-cpp-sdk-core/include/aws/core/external/cjson/cJSON.h

For projects that are using cJSON.h as well as linking aws-sdk-cpp, this is a breaking change as the structure of cJSON becomes redefined because the header guards used by the original library are different.

To reproduce this issue, simply use the original library within an application that is also using the aws-sdk-cpp.
https://github.com/DaveGamble/cJSON

To avoid this issue, it would be ideal if the typedef struct cJSON within the aws-cpp-sdk was renamed to something like cJSON_AS4CPP.

@scottpakula scottpakula added bug This issue is a bug. needs-triage This issue or PR still needs to be triaged. labels Dec 22, 2021
@KaibaLopez
Copy link
Contributor

Hi @scottpakula ,
I... am not sure I quite follow, so let me ask a few questions so I am on the same page here.
Basically you are trying to link the SDK and the CJSON libraries and they are having naming conflicts because the SDK already links but also redefines the CJSON library?

Also, and this is more for the benefit of anybody who might also get this issue before we come up with a solution, have you thought of any workarounds or are you currently blocked by this change?

@KaibaLopez KaibaLopez added response-requested Waiting on additional info and feedback. Will move to "closing-soon" in 10 days. and removed needs-triage This issue or PR still needs to be triaged. labels Dec 22, 2021
@KaibaLopez KaibaLopez self-assigned this Dec 22, 2021
@scottpakula
Copy link
Author

Thank for very much for the reply. That is correct, I have existing projects that have linked cJSON and the aws-sdk-cpp just fine for a while, but this recent change has broken my ability to compile, when these two libraries are used together. My workaround at the moment is to simply stick with 1.9.96.

The cause of this issue is that when cJSON.h was modified it also updated the header guard from:

#ifndef cJSON__h
#define cJSON__h

to:

#ifndef cJSON_AS4CPP__h
#define cJSON_AS4CPP__h

However, we get a redefinition error for the below data structure since typedef struct cJSON exists in both the original implementation of cJSON and the modified version of cJSON within the aws-sdk-cpp, and the header guards are different therefore cJSON will be defined twice.

/* The cJSON structure: */
typedef struct cJSON
{
    /* next/prev allow you to walk array/object chains. Alternatively, use GetArraySize/GetArrayItem/GetObjectItem */
    struct cJSON *next;
    struct cJSON *prev;
    /* An array or object item will have a child pointer pointing to a chain of the items in the array/object. */
    struct cJSON *child;

    /* The type of the item, as above. */
    int type;

    /* The item's string, if type==cJSON_AS4CPP_String  and type == cJSON_AS4CPP_Raw */
    char *valuestring;
    /* writing to valueint is DEPRECATED, use cJSON_AS4CPP_SetNumberValue instead */
    int valueint;
    /* The item's number, if type==cJSON_AS4CPP_Number */
    double valuedouble;

    /* The item's name string, if this item is the child of, or is in the list of subitems of an object. */
    char *string;
} cJSON;

Now the reason why this was changed according to the notes says:
"Renaming cJSON funcitons to avoid name clashes"

In my opinion, I would recommend that this change (2848c45) to cJSON within the aws-sdk-cpp be reverted and reviewed further. I'm not sure the clashes were being encountered; however, having to rename all the functions within the cJSON library seems a bit of an extreme fix. The way that the changes were made will cause conflicts with other projects that also use cJSON within their applications for the reasons noted above.

Let me know if this makes more sense and again thank you for your help.

@github-actions github-actions bot removed the response-requested Waiting on additional info and feedback. Will move to "closing-soon" in 10 days. label Dec 24, 2021
@scottpakula
Copy link
Author

It looks like an update was made that reverted the change. I will close this issue.

@github-actions
Copy link

⚠️COMMENT VISIBILITY WARNING⚠️

Comments on closed issues are hard for our team to see.
If you need more assistance, please either tag a team member or open a new issue that references this one.
If you wish to keep having a conversation with other community members under this issue feel free to do so.

@italorossi
Copy link

@scottpakula where did you see this update? Looks like the current version still have this problem.

@scottpakula scottpakula reopened this Apr 22, 2022
@scottpakula
Copy link
Author

scottpakula commented Apr 22, 2022

@italorossi you are correct, the issue still exists so I'm re-opening the issue. The way that I am working around this issue is by patching cJSON.h within the aws-sdk-cpp as part of my build process, and including both guards, and I got confused forgetting that I had this patch in place.

#ifndef cJSON__h
#define cJSON__h
#ifndef cJSON_AS4CPP__h
#define cJSON_AS4CPP__h

... code ..

#endif 
#endif 

This would be a quick fix, so that things continue to build, but this is a less than ideal solution to the problem as the library has been heavily forked within the aws-sdk-cpp project. This is making an assumption that typedef struct cJSON is the same in both projects, but now it's hard to know if that would/would not be true.

Some possible solutions might be:

  1. (Most Ideal) - Revert the changes made, and use cJSON as-is pinned to a known version and create a wrapper at a higher level to cJSON to handle conflicts within the C++ portions of the library.

  2. (Less ideal) Completely fork the project. Change "typedef struct cJSON" to "typedef struct cJSON_AS4CPP".

@sdavtaker
Copy link
Contributor

Hi @scottpakula, this will be solved as part of #1888 by depending directly in cJSON.

@jmklix jmklix added the p2 This is a standard priority issue label Mar 8, 2023
@davehorton
Copy link

I guess this is still open? I am experiencing the same issue described, using 1.11.143

@jmklix
Copy link
Member

jmklix commented Aug 18, 2023

Yes, this still open. We have yet to complete the changes in #1888

@jmklix jmklix added the pending-release This issue will be fixed by an approved PR that hasn't been released yet. label Aug 18, 2023
@ahmedyarub
Copy link

Any updates?

@jmklix
Copy link
Member

jmklix commented Jan 22, 2024

Sorry, but no updates. We are still working on this but I don't have a timeline for when this will be fixed

@jmklix jmklix added Cmake Cmake related submissions and removed pending-release This issue will be fixed by an approved PR that hasn't been released yet. labels Apr 19, 2024
@realsama
Copy link

I just encountered this also. No updates?

@JoSoReal
Copy link

JoSoReal commented Mar 5, 2025

The issue still persists in the latest version of library. are there any updates on a fix?

@scottpakula
Copy link
Author

Not sure if it helps but the workaround that I posted in 2021 still works. I'm still doing that

@JoSoReal
Copy link

JoSoReal commented Mar 5, 2025

Yes your workaround is still working fine. I just thought there might be an official fix for this in the meantime.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
bug This issue is a bug. Cmake Cmake related submissions p2 This is a standard priority issue
Projects
None yet
Development

No branches or pull requests

9 participants