CJSON_AS4CPP incompatable with original cJSON

scottpakula commented 2 years ago

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.

KaibaLopez commented 2 years ago

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?

scottpakula commented 2 years ago

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 (2848c4571c94b03bc558378440f091f2017ef7d3) 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.

scottpakula commented 2 years ago

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

github-actions[bot] commented 2 years ago

⚠️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 commented 2 years ago

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

scottpakula commented 2 years ago

@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:

(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.
(Less ideal) Completely fork the project. Change "typedef struct cJSON" to "typedef struct cJSON_AS4CPP".