Fix a few issues with the C generator (part 1 version 2) #14434

eafer · 2023-01-11T21:27:44Z

This is a second version of the pull request from #14388. The differences with the first version are:

this version uses the existing implementation of object_t, as requested by @ityuhui
the Config.cmake.in file was cleaned up (I think it showed up in the first place because I forgot to rebuild before running the script)
returnTypeIsEnum was abandoned and replaced with returnProperty, following the instructions by @wing328
the fourth patch from Fix a few issues with the C generator #14379 is also included now, because it got fixed

These patches fix a few compilation errors for this api's generated code, but they are not enough by themselves to get it to build. More patches will be submitted later.

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.

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, 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.

As required for a pull request, run the generate-samples.sh script and commit the changes.

eafer · 2023-01-11T21:28:17Z

@zhemant @michelealbano

wing328 · 2023-05-11T09:26:09Z

@ityuhui what do you think is the best way to proceed with this PR?

What about breaking it down into 4 smaller PRs to review and merge?

ityuhui · 2023-05-12T04:10:20Z

@wing328

Since it's been a long time since my last review, I will go through this PR recently to check if it can be merged or should break down into smaller PR.

ityuhui · 2023-05-15T09:47:27Z

I'm just wondering why this a0fa76d should change.

@wing328 Are these 2 commits 523e5de and 5a4a398 about returnProperty reviewed by you ?

I'm find with the rest of this PR.

eafer · 2023-05-15T23:30:57Z

Hi! Thank you for looking into this again.

I'm just wondering why this a0fa76d should change.

Because that's how *_parseFromJSON() and *_convertToJSON() are actually called by the rest of the generated code. For instance in the api-body.mustache template you have the following line:

        elementToReturn = {{{returnBaseType}}}_parseFromJSON({{classname}}localVarJSON);

For our instance_state model, this generates:

        elementToReturn = instance_state_parseFromJSON(ArmAPIlocalVarJSON);

But the *_parseFromJSON() function gets declared as:

arm_api_instance_state__e instance_state_instance_state_parseFromJSON(cJSON *instance_stateJSON);

So the build will not go well, with warnings such as this one:

warning: implicit declaration of function ‘instance_state_parseFromJSON’; did you mean instance_return_parseFromJSON’? [-Wimplicit-function-declaration]
 4045 |         elementToReturn = instance_state_parseFromJSON(ArmAPIlocalVarJSON);
      |                           ^~~~~~~~~~~~~~~~~~~~~~~~~~~~
      |                           instance_return_parseFromJSON

ityuhui · 2023-05-17T01:12:42Z

Thank you @eafer for the clarifacation.

I'm OK with this PR.

@wing328 If you agree the commits 523e5de and 5a4a398 about returnProperty, I think this PR can merge now.

eafer · 2023-05-23T17:56:29Z

Hey, is there anything more I can do here? Do you want me to walk you through those two patches?

ityuhui · 2023-05-24T02:10:52Z

I'm OK with this PR.

Just wait for @wing328 to review and merge.

eafer · 2023-05-31T22:26:44Z

Hey @wing328, is there anything more I can do to help here?

ityuhui · 2023-06-15T01:54:43Z

Hi @wing328

I think this PR can merge now.

wing328 · 2024-11-25T10:04:52Z

@eafer can you please resolve the merge conflicts when you've time?

sorry for the delay in getting it merged.

you may want to update modules/openapi-generator/src/test/resources/2_0/c/petstore.yaml with a few test schemas to cover this change.

eafer · 2024-11-26T02:56:46Z

@wing328 I did the backmerge you requested. I'll try to look into the test schemas later, but it's been a while and I'm struggling to get things to build again.

wing328 · 2024-11-27T10:16:06Z

updated samples via e3dfe90
and all tests passed

wing328 · 2024-11-27T10:16:37Z

just merged it. thanks again for the PR.

if i can find the time later, i'll update the c petstore spec to add some tests too.

The recent commit 47665aa ("Fix a few issues with the C generator (part 1 version 2) (OpenAPITools#14434)") didn't include any test schemas. Add them now, as requested: OpenAPITools#14434 (comment)

eafer · 2024-11-27T15:30:07Z

@wing328 Thanks! I just tried some changes for the test schemas and sent a new pull request with them: #20200. I hope it helps.

wing328 · 2024-11-27T15:56:59Z

@eafer thank you. will review and get it merged shortly

* [C] Add test schemas for the recent changes The recent commit 47665aa ("Fix a few issues with the C generator (part 1 version 2) (#14434)") didn't include any test schemas. Add them now, as requested: #14434 (comment) * Update samples * Fix sample update with missing files * More fixes for sample updates

eafer added 5 commits January 10, 2023 20:55

C: fix confusion of 'classname'/'classFilename'

a0fa76d

C: update the samples

7c46bc8

As required for a pull request, run the generate-samples.sh script and commit the changes.

eafer mentioned this pull request Jan 11, 2023

Fix a few issues with the C generator #14379

Closed

bookerdj mentioned this pull request Apr 20, 2023

[BUG] [C] Enum is attempted to be freed #15271

Open

6 tasks

wing328 added Issue: Bug Client: C labels May 11, 2023

ityuhui approved these changes May 24, 2023

View reviewed changes

wing328 added this to the 7.11.0 milestone Nov 25, 2024

Merge branch 'master' into c_api_fixes_1_v2

229d0c8

update samples

e3dfe90

wing328 merged commit 47665aa into OpenAPITools:master Nov 27, 2024
18 checks passed

eafer mentioned this pull request Nov 27, 2024

Tests for recent C fixes #20200

Merged

eafer mentioned this pull request Dec 3, 2024

Fix a few issues with the C generator (part 2) #20227

Merged

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

Fix a few issues with the C generator (part 1 version 2) #14434

Fix a few issues with the C generator (part 1 version 2) #14434

eafer commented Jan 11, 2023 •

edited

Loading

eafer commented Jan 11, 2023

wing328 commented May 11, 2023

ityuhui commented May 12, 2023

ityuhui commented May 15, 2023

eafer commented May 15, 2023

ityuhui commented May 17, 2023

eafer commented May 23, 2023

ityuhui commented May 24, 2023

eafer commented May 31, 2023

ityuhui commented Jun 15, 2023

wing328 commented Nov 25, 2024 •

edited

Loading

eafer commented Nov 26, 2024

wing328 commented Nov 27, 2024

wing328 commented Nov 27, 2024

eafer commented Nov 27, 2024

wing328 commented Nov 27, 2024

Fix a few issues with the C generator (part 1 version 2) #14434

Fix a few issues with the C generator (part 1 version 2) #14434

Conversation

eafer commented Jan 11, 2023 • edited Loading

eafer commented Jan 11, 2023

wing328 commented May 11, 2023

ityuhui commented May 12, 2023

ityuhui commented May 15, 2023

eafer commented May 15, 2023

ityuhui commented May 17, 2023

eafer commented May 23, 2023

ityuhui commented May 24, 2023

eafer commented May 31, 2023

ityuhui commented Jun 15, 2023

wing328 commented Nov 25, 2024 • edited Loading

eafer commented Nov 26, 2024

wing328 commented Nov 27, 2024

wing328 commented Nov 27, 2024

eafer commented Nov 27, 2024

wing328 commented Nov 27, 2024

eafer commented Jan 11, 2023 •

edited

Loading

wing328 commented Nov 25, 2024 •

edited

Loading