Repository navigation
Conversation
Boolean array items were added to the list with the raw int value (valueint). list_addElement takes a pointer, so the generated code does not compile under -Werror=int-conversion. The generated _free functions call free() on every list entry, and _convertToJSON reads each entry as *(cJSON_bool *)data, so a cast would not be correct either. Store a heap-allocated cJSON_bool, as the numeric branch already does. Applies to model-body.mustache and model-body-v7_20_0.mustache (used with declareNumberBooleanWithoutPointer=true).
Add a Flags schema with a boolean array to the C petstore spec. The sample builds (c and c-useJsonUnformatted) compile the generated model with -Werror=int-conversion, which fails on the previous template. Add a manual round-trip test in samples/client/petstore/c/unit-tests.
Contributor
There was a problem hiding this comment.
All reported issues were addressed across 18 files
Reply with feedback, questions, or to request a fix.
View guided diff | Re-trigger cubic
- The end: cleanup freed string and numeric list items but not boolean ones, so a parse error after a boolean item leaked it. - The unformatted template returned the create_internal result directly, skipping the end: cleanup when the allocation failed. - Regenerate c and c-useJsonUnformatted samples; add a failing-parse case to manual-flags.c (valgrind: 4 bytes leaked before, none after).
Contributor
There was a problem hiding this comment.
1 issue found across 15 files (changes from recent commits).
Prompt for AI agents (unresolved issues)
Check if these issues are valid — if so, understand the root cause of each and fix them. When an issue isn't valid or won't be fixed in this PR, reply in its thread with the reason and then resolve the thread. If appropriate, use sub-agents to investigate and fix each issue separately.
<file name="modules/openapi-generator/src/main/resources/C-libcurl/model-body-v7_20_0.mustache">
<violation number="1" location="modules/openapi-generator/src/main/resources/C-libcurl/model-body-v7_20_0.mustache:1031">
P3: On the new create-failure path, the `end:` cleanup doesn't free the `strdup`'d string/date/binary fields that were passed to `_create_internal`, so they leak when allocation fails. This predates the PR (the sibling `model-body.mustache` has the same gap), so it can be left for a follow-up.</violation>
</file>
Reply with feedback, questions, or to request a fix.
View guided diff | Re-trigger cubic
With NDEBUG the assert and the parse call are removed, so the error path would not run. Store the result before checking it.
Contributor
Author
|
cc @zhemant @ityuhui @michelealbano @eafer (C technical committee) |
This branch has not been deployed
This file contains hidden or 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
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.
cc @zhemant @ityuhui @michelealbano @eafer (C technical committee)
Problem
For an array of booleans (
type: array, items: {type: boolean}), the C-libcurldeserializer adds each item with the raw int value:
list_addElementtakes a pointer, so the generated code does not compile under-Werror=int-conversion. The generated_freealso callsfree()on every listentry, and
_convertToJSONreads each entry as*(cJSON_bool *)data, so a castalone would not be correct.
The same line exists in two templates:
model-body.mustache(default output)model-body-v7_20_0.mustache(used withdeclareNumberBooleanWithoutPointer=true)Real-world trigger: Discovered this issue while building the c library for Kubernetes 1.37 spec (
DeviceAttribute.bools). It failed to build before this fix.Fix
Heap-allocate each item, as the numeric branch already does, in both templates:
Testing
Flagsschema with a boolean array added to2_0/c/petstore.yaml.candc-useJsonUnformattedsamples build in CI with-Werror=int-conversion, so thegenerated model is compiled there. Both builds fail with the previous template (checked).
samples/client/petstore/c/unit-tests/manual-flags.c: round trip (parse, inspect, serialize,free). Passes with the fix.
PR checklist
Commit all changed files.
This is important, as CI jobs will verify all generator outputs of your HEAD commit as it would merge with master.
These must match the expectations made by your contribution.
You may regenerate an individual generator by passing the relevant config(s) as an argument to the script, for example
./bin/generate-samples.sh bin/configs/java*.IMPORTANT: Do NOT purge/delete any folders/files (e.g. tests) when regenerating the samples as manually written tests may be removed.
Summary by cubic
Fixes generated C-libcurl code for boolean array items so it compiles under
-Werror=int-conversionand matches how numeric items are handled.cJSON_boolpointers before being added to the list, in bothmodel-body.mustacheandmodel-body-v7_20_0.mustache.Flagsschema with a boolean array to the C petstore spec and includes a manual round-trip test.NDEBUG.Written for commit 467cf6e. Summary will update on new commits.