-
-
Notifications
You must be signed in to change notification settings - Fork 6.7k
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
C client generator improvements to support petstore #5837
Changes from 1 commit
File filter
Filter by extension
Conversations
Jump to
Diff view
Diff view
There are no files selected for viewing
Original file line number | Diff line number | Diff line change |
---|---|---|
@@ -0,0 +1,11 @@ | ||
#include <stdlib.h> | ||
#include <string.h> | ||
#include "../include/binary.h" | ||
|
||
binary_t* instantiate_binary_t(char* data, int len) { | ||
binary_t* ret = malloc(sizeof(struct binary_t)); | ||
ret->len=len; | ||
ret->data = malloc(len); | ||
memcpy(ret->data, data, len); | ||
return ret; | ||
} |
Original file line number | Diff line number | Diff line change |
---|---|---|
@@ -0,0 +1,14 @@ | ||
#ifndef INCLUDE_BINARY_H | ||
#define INCLUDE_BINARY_H | ||
|
||
#include <stdint.h> | ||
|
||
typedef struct binary_t | ||
{ | ||
uint8_t* data; | ||
unsigned int len; | ||
} binary_t; | ||
|
||
binary_t* instantiate_binary_t(char* data, int len); | ||
|
||
#endif // INCLUDE_BINARY_H |
Original file line number | Diff line number | Diff line change |
---|---|---|
|
@@ -11,6 +11,11 @@ | |
#include "../external/cJSON.h" | ||
#include "../include/list.h" | ||
#include "../include/keyValuePair.h" | ||
#include "../include/binary.h" | ||
|
||
char *base64encode(const void *b64_encode_this, int encode_this_many_bytes); | ||
|
||
char *base64decode(const void *b64_decode_this, int decode_this_many_bytes, int *decoded_bytes); | ||
|
||
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. The two functions There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Because I don't There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. OK. I got it. But users may want to use these 2 functions , but they do not want to include "binary.h" Moving to binary.h/binary.c is also acceptable for me. There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. ok There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Moving to binary.h/binary.c is also acceptable for me. There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. @michelealbano you dont need to include apiClient.h any where. You just need to import the main api's api.h this will import all the models required and supporting files for that API There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Thinking about it, I will move them to binary.h/binary.c. I |
||
typedef struct {{classname}}_t {{classname}}_t; | ||
|
||
|
Original file line number | Diff line number | Diff line change |
---|---|---|
@@ -1 +1 @@ | ||
4.3.0-SNAPSHOT | ||
4.3.1-SNAPSHOT |
Original file line number | Diff line number | Diff line change |
---|---|---|
@@ -0,0 +1,14 @@ | ||
#ifndef INCLUDE_BINARY_H | ||
#define INCLUDE_BINARY_H | ||
|
||
#include <stdint.h> | ||
|
||
typedef struct binary_t | ||
{ | ||
uint8_t* data; | ||
unsigned int len; | ||
} binary_t; | ||
|
||
binary_t* instantiate_binary_t(char* data, int len); | ||
|
||
#endif // INCLUDE_BINARY_H |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Why this fake encode/decode is needed ? If uses do not define
-DOPENSSL
, but use the functionbase64decode
orbase64encode
, they will not get the result that they want.I think if the users want to use the function
base64decode
orbase64encode
defined by openapi-generator/c-libcurl , they must define -DOPENSSL in CMakefile.There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I didn't get it. What if
-DOPENSSL
is not defined? What do you intend to happen in that case? I didn't have it defined, and the function "returned" without issuing areturn
statement.There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
e.g.
If uses do not define
-DOPENSSL
, but use the functionbase64encode
, the returnedchar *
is still equal to the original string, not a base64 encoded string.There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I thought that was idea, and that's the code I implemented. The original code did nothing (not even
return
a buffer).There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Yes. I know.
So I suggest adding a warning message here if this code is needed:
e.g.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I would suggest returning NULL so the whole API call exits with an error. this is because we use strlen to get the length of the data when we add data to curl handle. This will break if the data has binary(may include null termination). So for a safer side, I would suggest to return NULL or mandate OpenSSL installation as a prerequisite.
Also if the server is expecting a base64 encoded string then we cannot send unencoded data.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
What if I don't have OpenSSL, and I just want to generate unencrypted APIs?
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
If you don't have OpenSSL then you cannot send or receive binary data.
Also, I didn't get what you mean by unencrypted API? Because OpenSSL is used only for encoding binary data. It is not used for encryption.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Ok.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I updated also
CMakeLists.txt
, but I am not an expert oncmake
.