Fix a few issues with the C generator (part 1)#14388
Fix a few issues with the C generator (part 1)#14388eafer wants to merge 4 commits intoOpenAPITools:masterfrom
Conversation
Implement an object type that simply holds a cJSON object inside. This patch was originally based on an old version of openapi where object_t was broken, so it's probably unnecessary now. The only difference is that the internal representation is as a cJSON object, not a string.
I'm guessing that enums have not been used much with the C generator before, because they always seem to produce code that doesn't build, or that tries to free them after use. This patch fixes all the problems we've encountered so far, except for those that need checking the return type. I'll come back to that later.
As required for a pull request, run the generate-samples.sh script and commit the changes.
| @@ -0,0 +1,5 @@ | |||
| /* | |||
There was a problem hiding this comment.
If you need add header or source files, you can add a "PreTarget.cmake", e.g. https://github.com/kubernetes-client/c/blob/master/kubernetes/PreTarget.cmake
There was a problem hiding this comment.
The any_type.h header was not our invention, the generator sometimes puts #include "any_type.h" lines in the code, so this was needed to get it to build. A better solution would be to check why these lines are getting added in the first place, but either way this looks like a bug, not something that should be fixed by the user.
By the way this is the api we are working with, in case you want to check why we are running into different problems than you.
There was a problem hiding this comment.
Thank you for the clarification. I'll go to check it.
There was a problem hiding this comment.
This header is generated if the api yaml has anyof present other than that of enums. Currently anyof, oneof and allof are not handled. Anyof is handled but only for enums, there is a hack in java code for this.
There was a problem hiding this comment.
The problem is that the #include lines are added, but the header itself is not generated (at least for our api), so the build is broken. I got it to work by adding an empty any_type.h header, but maybe you know a better way?
| @@ -0,0 +1,5 @@ | |||
| @PACKAGE_INIT@ | |||
There was a problem hiding this comment.
If a project has its own "Config.cmake.in", your template file will replace it...
There was a problem hiding this comment.
There was a problem hiding this comment.
Weird, this pull request doesn't seem to include any cmake stuff. Any idea why this is happening?
There was a problem hiding this comment.
It is, probably you need to manually remove it from this PR.
|
|
||
| typedef struct object_t { | ||
| void *temporary; | ||
| cJSON *json; |
There was a problem hiding this comment.
Why void * needs to be changed to cJSON *? May I know what requirements object.h and object.c cannot meet?
There was a problem hiding this comment.
None at all, I explained that in the commit message. This patch was written at a time when object_t was broken, before you fixed it yourself. So it was easier for us to just keep working with our version, which we had already tested. We didn't think anyone would mind, since so many changes are needed anyway, but I can use your version of object_t if you prefer.
There was a problem hiding this comment.
It's good to keep the current code. Actually this is not my original. I took the code from 166de17 and found it to work fine. So let's keep the code.
There was a problem hiding this comment.
If needed we could modify it as a union so we can accommodate int, char, void* data types as well
|
I submitted a new version of this pull request, so I guess this one can be closed. |
3 participants
As requested, I'm splitting the first three patches from #14379 into a new pull request to make them easier to review. These fix a few compilation errors for the generated code, but they are not enough by themselves to get it to build.