Fix a few issues with the C generator #14379
Closed
Conversation
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
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.
Currently, there doesn't seem to be any way to check if a function returns an enum inside mustache, so when that happens the generated code has broken return types and doesn't build. Extend CodegenOperation to implement a 'returnTypeIsEnum' check for the templates, and use it to fix the bugs.
If the body provided for the api request is a just a string itself, don't try to convert it to JSON, simply submit the string.
Before making an api call, reset apiClient->response_code to zero. That will protect us from checking stale values if the curl request fails.
Check early on that the arguments are not null, to prevent crashes on strdup() calls.
Check if the api call returned an error before attempting to parse the reply as the expected type.
Continue extending CodegenOperation to deal with binary and integer return types.
With this change, the bodyParameters array can also be binary, so pass its length around instead of relying on strlen().
The behaviour of *_free() doesn't match *_create(), so the user should avoid using them together. But they still need *_free() to clean up library-allocated objects, so add a _library_owned flag to each struct as an attempt to tell them apart. This isn't perfect though, because the user may neglect to zero the field, but they would still see a warning once in a while so it serves it purpose. To prevent the new deprecation warnings (intended for the user) from showing up during the library build itself, define a new family of *_create_internal() functions, and turn *_create() into simple wrappers.
As required for a pull request, run the generate-samples.sh script and commit the changes.
|
It seems that I broke something for the petstore sample, I've never tested that I'm afraid. I'll see if I can fix it. |
|
Thanks for the PR ! |
wing328
reviewed
Jan 5, 2023
...penapi-generator/src/main/java/org/openapitools/codegen/languages/CLibcurlClientCodegen.java
Show resolved
Hide resolved
|
@eafer thanks for the PR. What about breaking down this PR into smaller one for easier review and merge? |
|
cc @zhemant (2018/11) @ityuhui (2019/12) @michelealbano (2020/03) |
|
Ok, since you have an objection to the 4th patch, I'll start by resubmitting the first 3. |
ityuhui
reviewed
Jan 6, 2023
|
|
||
| cmake_policy(SET CMP0063 NEW) | ||
|
|
||
| set(CMAKE_C_VISIBILITY_PRESET default) | ||
| set(CMAKE_CXX_VISIBILITY_PRESET default) |
There was a problem hiding this comment.
Some developers use a C++ compiler to compile this project. Will this change break it?
There was a problem hiding this comment.
We can discuss this in your new PR. Just putting it here for your consideration.
ityuhui
reviewed
Jan 6, 2023
| @@ -435,8 +453,8 @@ void apiClient_invoke(apiClient_t *apiClient, | |||
| curl_easy_setopt(handle, CURLOPT_SSL_VERIFYHOST, 2L); | |||
| } | |||
| } else { | |||
| curl_easy_setopt(handle, CURLOPT_SSL_VERIFYPEER, 0L); | |||
| curl_easy_setopt(handle, CURLOPT_SSL_VERIFYHOST, 0L); | |||
| curl_easy_setopt(handle, CURLOPT_SSL_VERIFYPEER, 1L); | |||
There was a problem hiding this comment.
apiClient->sslConfig should be used to enable and configure TLS.
There was a problem hiding this comment.
We can discuss this in your new PR. Just putting it here for your consideration.
There was a problem hiding this comment.
Sure, but isn't it better to default to the safer option? If you disagree, I guess we can just keep applying this patch internally.
eafer
added a commit
to eafer/openapi-generator
that referenced
this pull request
Jan 11, 2023
Currently, the C templates never check if a function returns an enum inside mustache, so when that happens the generated code has broken return types and doesn't build. I originally tried to fix this by extending CodegenOperation to implement a 'returnTypeIsEnum' check, but William Cheng suggested[1] that I use the existing 'returnProperty' instead: OpenAPITools#14379 (comment) So do that.
5 tasks
wing328
added a commit
that referenced
this pull request
Nov 27, 2024
* C: add a template for an empty any_type.h header Some generated C apis fail to build because the source files get '#include "any_type.h"' lines, but no such header gets generated. As a simple fix, add a new template for an empty file with that name. This is enough to fix the problem for us, because all the generic type stuff is handled by object_t. * C: fix enums 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. * C: fix confusion of 'classname'/'classFilename' * C: fix issues with returned enums Currently, the C templates never check if a function returns an enum inside mustache, so when that happens the generated code has broken return types and doesn't build. I originally tried to fix this by extending CodegenOperation to implement a 'returnTypeIsEnum' check, but William Cheng suggested[1] that I use the existing 'returnProperty' instead: #14379 (comment) So do that. * C: update the samples As required for a pull request, run the generate-samples.sh script and commit the changes. * update samples --------- Co-authored-by: William Cheng <[email protected]>
This was referenced Dec 3, 2024
This was referenced Dec 11, 2024
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.
This suggestion is invalid because no changes were made to the code.
Suggestions cannot be applied while the pull request is closed.
Suggestions cannot be applied while viewing a subset of changes.
Only one suggestion per line can be applied in a batch.
Add this suggestion to a batch that can be applied as a single commit.
Applying suggestions on deleted lines is not supported.
You must change the existing code in this line in order to create a valid suggestion.
Outdated suggestions cannot be applied.
This suggestion has been applied or marked resolved.
Suggestions cannot be applied from pending reviews.
Suggestions cannot be applied on multi-line comments.
Suggestions cannot be applied while the pull request is queued to merge.
Suggestion cannot be applied right now. Please check back later.
Hi @zhemant @ityuhui @michelealbano, at my company (Corellium) we've been using your C generator to produce this api: https://github.com/ARM-software/avh-api. Several changes were needed to get the code to build, especially when enums are involved, so I'm guessing it's not heavily used by anyone else. We are hoping you will be interested in picking up our changes. Thank you for all your work, and I apologize in advance if I messed up the pull request somehow, I'll be happy to make any changes you require.