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
Remove curl_mimepart object from UserDefined structure when CURL_DISABLE_MIME flag is active #12948
Conversation
6776bd7
to
d5742e8
Compare
6e81fb6
to
baae225
Compare
lib/imap.c
Outdated
@@ -1513,10 +1522,10 @@ static CURLcode imap_done(struct Curl_easy *data, CURLcode status, | |||
} | |||
else if(!data->set.connect_only && !imap->custom && | |||
(imap->uid || imap->mindex || data->state.upload || | |||
data->set.mimepost.kind != MIMEKIND_NONE)) { | |||
IMAP_MIMEKIND_IS_NOT_NONE(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.
couldn't this be a single shorter macro somewhere like urldata and make it different for when mime is disabled like
#ifndef CURL_DISABLE_MIME
#define IS_MIME_POST(a) ((a)->set.mimepost.kind != MIMEKIND_NONE)
#else
#define IS_MIME_POST(a) FALSE
#endif
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've moved the macro definition to urldata.h.
…BLE_MIME flag is active. Reduce size of UserDefined structure. Also remove unreachable code: when CURL_DISABLE_MIME is set, httpreq can never have HTTPREQ_POST_MIME value and the same goes for the CURL_DISABLE_FORM_API flag and the HTTPREQ_POST_FORM value
baae225
to
aeab724
Compare
Did you test this? Though it passes the CI (since we don't test CURL_DISABLE_MIME) there are warnings and it fails to compile:
formdata.c(726): size_t nread = Curl_mime_read(buffer, 1, sizeof(buffer), &toppart); mime.h(169): #define Curl_mime_read NULL It should be a function-like macro such as #define Curl_mime_read(a,b,c,d) NULL but that causes different errors because of code that passes the function as a pointer even when CURL_DISABLE_MIME, which doesn't seem right. for example http.c(2582): data->state.fread_func = (curl_read_callback) Curl_mime_read; |
Yes, I saw the warning and error. They ocur, when user defines CURL_DISABLE_MIME flag without CURL_DISABLE_FORM_API. In the project I'm working on I have both of these flags applied and only tested this scenario. This problems occur also without this PR. The problem with Curl_mime_contenttype is easy to solve. Simply write
in mime.h in section where CURL_DISABLE_MIME is defined. More complex task is solve the problem with Curl_mime_read. I don't quite know how to solve it. It is the public function. I didn't analyze what is the right behavour when CURL_DISABLE_MIME is defined. |
If I understand code correctly the Curl_getformdata function return CURLE_OUT_OF_MEMORY and also curl_formget return the same error code. Maybe the solution is:
Or better return CURL_FORMADD_DISABLED code. |
This case is a nonsense since the form api uses the mime api. There already is a guard against it in configure.ac and a test for it ought to be be added in CMakeLists.txt by someone who masters cmake. Introducing macros and/or conditionals for that situation only decreases the code readability. If the ultimate goal of this PR is to spare room in the UserDefined structure, I've had a good result by simply shortening the curl_mimedata structure to an empty one:
Some code bytes are saved by this PR though. |
Thanks! |
Reduce size of UserDefined structure. For the x86 architecture, the structure size decreased by 368 bytes, for x64 440 bytes. It is therefore a significant reduction in the size of the structure.
Also remove unreachable code: when CURL_DISABLE_MIME is set, httpreq can never have HTTPREQ_POST_MIME value and the same goes for the CURL_DISABLE_FORM_API flag and the HTTPREQ_POST_FORM value.
As for the name of the function I created in smtp.c and imap.c I am open to suggestions.