Skip to content

Remove curl_mimepart object from UserDefined structure when CURL_DISABLE_MIME flag is active - #12948

Closed
MAntoniak wants to merge 1 commit into
curl:masterfrom
MAntoniak:remove_mime
Closed

MAntoniak wants to merge 1 commit into
curl:masterfrom
MAntoniak:remove_mime

Conversation

@MAntoniak

Copy link
Copy Markdown
Contributor

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.

@MAntoniak
MAntoniak marked this pull request as draft February 16, 2024 07:07
@MAntoniak
MAntoniak force-pushed the remove_mime branch 2 times, most recently from 6e81fb6 to baae225 Compare February 17, 2024 20:24
@MAntoniak
MAntoniak marked this pull request as ready for review February 17, 2024 22:45
Comment thread lib/imap.c Outdated

Copy link
Copy Markdown
Member

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

Copy link
Copy Markdown
Contributor Author

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
@jay

jay commented Feb 22, 2024

Copy link
Copy Markdown
Member

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:

..\..\..\..\lib\formdata.c(575): warning C4013: 'Curl_mime_contenttype' undefined; assuming extern returning int
..\..\..\..\lib\formdata.c(575): warning C4047: '=' : 'const char *' differs in levels of indirection from 'int'
..\..\..\..\lib\formdata.c(726): error C2064: term does not evaluate to a function taking 305 arguments

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;

@MAntoniak

Copy link
Copy Markdown
Contributor Author

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

#define Curl_mime_contenttype NULL

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.

@MAntoniak

MAntoniak commented Feb 22, 2024 •

Copy link
Copy Markdown
Contributor Author

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:

--- a/src/lib/curl/lib/formdata.c
+++ b/src/lib/curl/lib/formdata.c
@@ -712,6 +712,7 @@ CURLFORMcode curl_formadd(struct curl_httppost **httppost,
 int curl_formget(struct curl_httppost *form, void *arg,
                  curl_formget_callback append)
 {
+#ifndef CURL_DISABLE_MIME
   CURLcode result;
   curl_mimepart toppart;
 
@@ -737,6 +738,9 @@ int curl_formget(struct curl_httppost *form, void *arg,
 
   Curl_mime_cleanpart(&toppart);
   return (int) result;
+#else
+  return CURLE_OUT_OF_MEMORY;
+#endif
 }

Or better return CURL_FORMADD_DISABLED code.

@jay

jay commented Feb 24, 2024

Copy link
Copy Markdown
Member

@monnerat

@monnerat

Copy link
Copy Markdown
Contributor

They ocur, when user defines CURL_DISABLE_MIME flag without CURL_DISABLE_FORM_API

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:

-- a/lib/mime.h
+++ b/lib/mime.h
@@ -108,6 +108,9 @@ struct curl_mime {
 
 /* A mime part. */
 struct curl_mimepart {
+#ifdef CURL_DISABLE_MIME
+  char dummy[0];
+#else
   curl_mime *parent;               /* Parent mime structure. */
   curl_mimepart *nextpart;         /* Forward linked list. */
   enum mimekind kind;              /* The part kind. */
@@ -128,6 +131,7 @@ struct curl_mimepart {
   const struct mime_encoder *encoder; /* Content data encoder. */
   struct mime_encoder_state encstate; /* Data encoder state. */
   size_t lastreadstatus;           /* Last read callback returned status. */
+#endif
 };
 
 CURLcode Curl_mime_add_header(struct curl_slist **slp, const char *fmt, ...)

Some code bytes are saved by this PR though.

@bagder bagder closed this in e26c362 Feb 26, 2024
@bagder

bagder commented Feb 26, 2024

Copy link
Copy Markdown
Member

Thanks!

@MAntoniak
MAntoniak deleted the remove_mime branch February 26, 2024 21:36
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Development

Successfully merging this pull request may close these issues.

4 participants