Skip to content

Commit

Permalink
Fix a few issues with the C generator (part 8) (#20378)
Browse files Browse the repository at this point in the history
* [C] Deprecate *_create() to avoid *_free() confusion

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

* Update samples

* add eafer to c technical committee

---------

Co-authored-by: William Cheng <[email protected]>
  • Loading branch information
eafer and wing328 authored Jan 6, 2025
1 parent 85c81be commit e154903
Show file tree
Hide file tree
Showing 35 changed files with 450 additions and 52 deletions.
2 changes: 1 addition & 1 deletion README.md
Original file line number Diff line number Diff line change
Expand Up @@ -1210,7 +1210,7 @@ If you want to join the committee, please kindly apply by sending an email to te
| Android | @jaz-ah (2017/09) |
| Apex | |
| Bash | @frol (2017/07) @bkryza (2017/08) @kenjones-cisco (2017/09) |
| C | @zhemant (2018/11) @ityuhui (2019/12) @michelealbano (2020/03) |
| C | @zhemant (2018/11) @ityuhui (2019/12) @michelealbano (2020/03) @eafer (2024/12) |
| C++ | @ravinikam (2017/07) @stkrwork (2017/07) @etherealjoy (2018/02) @martindelille (2018/03) @muttleyxd (2019/08) |
| C# | @mandrean (2017/08) @shibayan (2020/02) @Blackclaws (2021/03) @lucamazzanti (2021/05) @iBicha (2023/07) |
| Clojure | |
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -119,7 +119,7 @@ char* {{classname}}_{{name}}_ToString({{projectName}}_{{classVarName}}_{{enumNam
{{/isContainer}}
{{/vars}}

{{classname}}_t *{{classname}}_create(
static {{classname}}_t *{{classname}}_create_internal(
{{#vars}}
{{^isContainer}}
{{^isPrimitiveType}}
Expand Down Expand Up @@ -205,14 +205,103 @@ char* {{classname}}_{{name}}_ToString({{projectName}}_{{classVarName}}_{{enumNam
{{classname}}_local_var->{{{name}}} = {{{name}}};
{{/vars}}

{{classname}}_local_var->_library_owned = 1;
return {{classname}}_local_var;
}

__attribute__((deprecated)) {{classname}}_t *{{classname}}_create(
{{#vars}}
{{^isContainer}}
{{^isPrimitiveType}}
{{#isModel}}
{{#isEnum}}
{{projectName}}_{{classVarName}}_{{enumName}}_e {{name}}{{^-last}},{{/-last}}
{{/isEnum}}
{{^isEnum}}
{{datatype}}_t *{{name}}{{^-last}},{{/-last}}
{{/isEnum}}
{{/isModel}}
{{^isModel}}
{{^isFreeFormObject}}
{{^isEnum}}
{{datatype}}_t *{{name}}{{^-last}},{{/-last}}
{{/isEnum}}
{{#isEnum}}
{{projectName}}_{{dataType}}_{{enumName}}_e {{name}}{{^-last}},{{/-last}}
{{/isEnum}}
{{/isFreeFormObject}}
{{/isModel}}
{{#isUuid}}
{{datatype}} *{{name}}{{^-last}},{{/-last}}
{{/isUuid}}
{{#isEmail}}
{{datatype}} *{{name}}{{^-last}},{{/-last}}
{{/isEmail}}
{{#isFreeFormObject}}
{{datatype}}_t *{{name}}{{^-last}},{{/-last}}
{{/isFreeFormObject}}
{{/isPrimitiveType}}
{{#isPrimitiveType}}
{{#isNumeric}}
{{datatype}} {{name}}{{^-last}},{{/-last}}
{{/isNumeric}}
{{#isBoolean}}
{{datatype}} {{name}}{{^-last}},{{/-last}}
{{/isBoolean}}
{{#isEnum}}
{{#isString}}
{{projectName}}_{{classVarName}}_{{enumName}}_e {{name}}{{^-last}},{{/-last}}
{{/isString}}
{{/isEnum}}
{{^isEnum}}
{{#isString}}
{{datatype}} *{{name}}{{^-last}},{{/-last}}
{{/isString}}
{{/isEnum}}
{{#isByteArray}}
{{datatype}} *{{name}}{{^-last}},{{/-last}}
{{/isByteArray}}
{{#isBinary}}
{{datatype}} {{name}}{{^-last}},{{/-last}}
{{/isBinary}}
{{#isDate}}
{{datatype}} *{{name}}{{^-last}},{{/-last}}
{{/isDate}}
{{#isDateTime}}
{{datatype}} *{{name}}{{^-last}},{{/-last}}
{{/isDateTime}}
{{/isPrimitiveType}}
{{/isContainer}}
{{#isContainer}}
{{#isArray}}
{{#isPrimitiveType}}
{{datatype}}_t *{{name}}{{^-last}},{{/-last}}
{{/isPrimitiveType}}
{{^isPrimitiveType}}
{{datatype}}_t *{{name}}{{^-last}},{{/-last}}
{{/isPrimitiveType}}
{{/isArray}}
{{#isMap}}
{{datatype}} {{name}}{{^-last}},{{/-last}}
{{/isMap}}
{{/isContainer}}
{{/vars}}
) {
return {{classname}}_create_internal (
{{#vars}}
{{name}}{{^-last}},{{/-last}}
{{/vars}}
);
}

void {{classname}}_free({{classname}}_t *{{classname}}) {
if(NULL == {{classname}}){
return ;
}
if({{classname}}->_library_owned != 1){
fprintf(stderr, "WARNING: %s() does NOT free objects allocated by the user\n", "{{classname}}_free");
return ;
}
listEntry_t *listEntry;
{{#vars}}
{{^isContainer}}
Expand Down Expand Up @@ -859,7 +948,7 @@ fail:

{{/vars}}

{{classname}}_local_var = {{classname}}_create (
{{classname}}_local_var = {{classname}}_create_internal (
{{#vars}}
{{^isContainer}}
{{^isPrimitiveType}}
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -154,9 +154,10 @@ typedef struct {{classname}}_t {
{{/isContainer}}
{{/vars}}

int _library_owned; // Is the library responsible for freeing this object?
} {{classname}}_t;

{{classname}}_t *{{classname}}_create(
__attribute__((deprecated)) {{classname}}_t *{{classname}}_create(
{{#vars}}
{{^isContainer}}
{{^isPrimitiveType}}
Expand Down
20 changes: 18 additions & 2 deletions samples/client/petstore/c-useJsonUnformatted/model/api_response.c
Original file line number Diff line number Diff line change
Expand Up @@ -5,7 +5,7 @@



api_response_t *api_response_create(
static api_response_t *api_response_create_internal(
int code,
char *type,
char *message
Expand All @@ -18,14 +18,30 @@ api_response_t *api_response_create(
api_response_local_var->type = type;
api_response_local_var->message = message;

api_response_local_var->_library_owned = 1;
return api_response_local_var;
}

__attribute__((deprecated)) api_response_t *api_response_create(
int code,
char *type,
char *message
) {
return api_response_create_internal (
code,
type,
message
);
}

void api_response_free(api_response_t *api_response) {
if(NULL == api_response){
return ;
}
if(api_response->_library_owned != 1){
fprintf(stderr, "WARNING: %s() does NOT free objects allocated by the user\n", "api_response_free");
return ;
}
listEntry_t *listEntry;
if (api_response->type) {
free(api_response->type);
Expand Down Expand Up @@ -113,7 +129,7 @@ api_response_t *api_response_parseFromJSON(cJSON *api_responseJSON){
}


api_response_local_var = api_response_create (
api_response_local_var = api_response_create_internal (
code ? code->valuedouble : 0,
type && !cJSON_IsNull(type) ? strdup(type->valuestring) : NULL,
message && !cJSON_IsNull(message) ? strdup(message->valuestring) : NULL
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -23,9 +23,10 @@ typedef struct api_response_t {
char *type; // string
char *message; // string

int _library_owned; // Is the library responsible for freeing this object?
} api_response_t;

api_response_t *api_response_create(
__attribute__((deprecated)) api_response_t *api_response_create(
int code,
char *type,
char *message
Expand Down
18 changes: 16 additions & 2 deletions samples/client/petstore/c-useJsonUnformatted/model/category.c
Original file line number Diff line number Diff line change
Expand Up @@ -5,7 +5,7 @@



category_t *category_create(
static category_t *category_create_internal(
long id,
char *name
) {
Expand All @@ -16,14 +16,28 @@ category_t *category_create(
category_local_var->id = id;
category_local_var->name = name;

category_local_var->_library_owned = 1;
return category_local_var;
}

__attribute__((deprecated)) category_t *category_create(
long id,
char *name
) {
return category_create_internal (
id,
name
);
}

void category_free(category_t *category) {
if(NULL == category){
return ;
}
if(category->_library_owned != 1){
fprintf(stderr, "WARNING: %s() does NOT free objects allocated by the user\n", "category_free");
return ;
}
listEntry_t *listEntry;
if (category->name) {
free(category->name);
Expand Down Expand Up @@ -87,7 +101,7 @@ category_t *category_parseFromJSON(cJSON *categoryJSON){
}


category_local_var = category_create (
category_local_var = category_create_internal (
id ? id->valuedouble : 0,
name && !cJSON_IsNull(name) ? strdup(name->valuestring) : NULL
);
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -22,9 +22,10 @@ typedef struct category_t {
long id; //numeric
char *name; // string

int _library_owned; // Is the library responsible for freeing this object?
} category_t;

category_t *category_create(
__attribute__((deprecated)) category_t *category_create(
long id,
char *name
);
Expand Down
18 changes: 16 additions & 2 deletions samples/client/petstore/c-useJsonUnformatted/model/mapped_model.c
Original file line number Diff line number Diff line change
Expand Up @@ -5,7 +5,7 @@



MappedModel_t *MappedModel_create(
static MappedModel_t *MappedModel_create_internal(
int another_property,
char *uuid_property
) {
Expand All @@ -16,14 +16,28 @@ MappedModel_t *MappedModel_create(
MappedModel_local_var->another_property = another_property;
MappedModel_local_var->uuid_property = uuid_property;

MappedModel_local_var->_library_owned = 1;
return MappedModel_local_var;
}

__attribute__((deprecated)) MappedModel_t *MappedModel_create(
int another_property,
char *uuid_property
) {
return MappedModel_create_internal (
another_property,
uuid_property
);
}

void MappedModel_free(MappedModel_t *MappedModel) {
if(NULL == MappedModel){
return ;
}
if(MappedModel->_library_owned != 1){
fprintf(stderr, "WARNING: %s() does NOT free objects allocated by the user\n", "MappedModel_free");
return ;
}
listEntry_t *listEntry;
if (MappedModel->uuid_property) {
free(MappedModel->uuid_property);
Expand Down Expand Up @@ -87,7 +101,7 @@ MappedModel_t *MappedModel_parseFromJSON(cJSON *MappedModelJSON){
}


MappedModel_local_var = MappedModel_create (
MappedModel_local_var = MappedModel_create_internal (
another_property ? another_property->valuedouble : 0,
uuid_property && !cJSON_IsNull(uuid_property) ? strdup(uuid_property->valuestring) : NULL
);
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -22,9 +22,10 @@ typedef struct MappedModel_t {
int another_property; //numeric
char *uuid_property; // string

int _library_owned; // Is the library responsible for freeing this object?
} MappedModel_t;

MappedModel_t *MappedModel_create(
__attribute__((deprecated)) MappedModel_t *MappedModel_create(
int another_property,
char *uuid_property
);
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -5,7 +5,7 @@



model_with_set_propertes_t *model_with_set_propertes_create(
static model_with_set_propertes_t *model_with_set_propertes_create_internal(
list_t *tag_set,
list_t *string_set
) {
Expand All @@ -16,14 +16,28 @@ model_with_set_propertes_t *model_with_set_propertes_create(
model_with_set_propertes_local_var->tag_set = tag_set;
model_with_set_propertes_local_var->string_set = string_set;

model_with_set_propertes_local_var->_library_owned = 1;
return model_with_set_propertes_local_var;
}

__attribute__((deprecated)) model_with_set_propertes_t *model_with_set_propertes_create(
list_t *tag_set,
list_t *string_set
) {
return model_with_set_propertes_create_internal (
tag_set,
string_set
);
}

void model_with_set_propertes_free(model_with_set_propertes_t *model_with_set_propertes) {
if(NULL == model_with_set_propertes){
return ;
}
if(model_with_set_propertes->_library_owned != 1){
fprintf(stderr, "WARNING: %s() does NOT free objects allocated by the user\n", "model_with_set_propertes_free");
return ;
}
listEntry_t *listEntry;
if (model_with_set_propertes->tag_set) {
list_ForEach(listEntry, model_with_set_propertes->tag_set) {
Expand Down Expand Up @@ -146,7 +160,7 @@ model_with_set_propertes_t *model_with_set_propertes_parseFromJSON(cJSON *model_
}


model_with_set_propertes_local_var = model_with_set_propertes_create (
model_with_set_propertes_local_var = model_with_set_propertes_create_internal (
tag_set ? tag_setList : NULL,
string_set ? string_setList : NULL
);
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -23,9 +23,10 @@ typedef struct model_with_set_propertes_t {
list_t *tag_set; //nonprimitive container
list_t *string_set; //primitive container

int _library_owned; // Is the library responsible for freeing this object?
} model_with_set_propertes_t;

model_with_set_propertes_t *model_with_set_propertes_create(
__attribute__((deprecated)) model_with_set_propertes_t *model_with_set_propertes_create(
list_t *tag_set,
list_t *string_set
);
Expand Down
Loading

0 comments on commit e154903

Please sign in to comment.