lib-http: Clarify http_client_request_*_payload() API and minor change to it

The earlier behavior was pretty confusing, and potentially could have caused
double-freeing memory in some situations. Now it's clear that req is set to NULL
always when the request is finished, regardless of whether it has any references left.

Changed http_client_request_finish_payload() to return 0 on success instead of 1.
This could have been left alone, but it's unlikely that there is any code outside
Dovecot core that calls it and this way is cleaner.

diff --git a/src/lib-http/http-client-request.c b/src/lib-http/http-client-request.c
index 2c985d7d13080c65f3abc9d087c7570b9cd87772..492a22887eb91ee54f7947dac96f110998a3e1e7 100644 (file)
--- a/src/lib-http/http-client-request.c
+++ b/src/lib-http/http-client-request.c
@@ -711,14 +711,30 @@ http_client_request_continue_payload(struct http_client_request **_req,
 int http_client_request_send_payload(struct http_client_request **_req,
        const unsigned char *data, size_t size)
 {
+       struct http_client_request *req = *_req;
+       int ret;
+
        i_assert(data != NULL);
 
-       return http_client_request_continue_payload(_req, data, size);
+       ret = http_client_request_continue_payload(&req, data, size);
+       if (ret < 0)
+               *_req = NULL;
+       else {
+               i_assert(ret == 0);
+               i_assert(req != NULL);
+       }
+       return ret;
 }
 
 int http_client_request_finish_payload(struct http_client_request **_req)
 {
-       return http_client_request_continue_payload(_req, NULL, 0);
+       struct http_client_request *req = *_req;
+       int ret;
+
+       *_req = NULL;
+       ret = http_client_request_continue_payload(&req, NULL, 0);
+       i_assert(ret != 0);
+       return ret < 0 ? -1 : 0;
 }
 
 static void http_client_request_payload_input(struct http_client_request *req)

diff --git a/src/lib-http/http-client.h b/src/lib-http/http-client.h
index 016a05dcff0e87b195b2b92a707f695585f96f7a..9f59c58d04dbdd6651a42891d634580ba61573c0 100644 (file)
--- a/src/lib-http/http-client.h
+++ b/src/lib-http/http-client.h
@@ -236,9 +236,13 @@ void http_client_request_set_destroy_callback(struct http_client_request *req,
 
 /* submits request and blocks until provided payload is sent. Multiple calls
    are allowed; payload transmission is ended with
-   http_client_request_finish_payload(). */
+   http_client_request_finish_payload(). If the sending fails, returns -1
+   and sets req=NULL to indicate that the request was freed, otherwise
+   returns 0 and req is unchanged. */
 int http_client_request_send_payload(struct http_client_request **req,
        const unsigned char *data, size_t size);
+/* Finish sending the payload. Always frees req and sets it to NULL.
+   Returns 0 on success, -1 on error. */
 int http_client_request_finish_payload(struct http_client_request **req);
 
 void http_client_request_start_tunnel(struct http_client_request *req,

diff --git a/src/plugins/fts-solr/solr-connection.c b/src/plugins/fts-solr/solr-connection.c
index 54abf57a452c1af13dee8e150ed89ceed561acaa..1eca106876932582d2ca8b69981c3bad23373f95 100644 (file)
--- a/src/plugins/fts-solr/solr-connection.c
+++ b/src/plugins/fts-solr/solr-connection.c
@@ -532,7 +532,7 @@ int solr_connection_post_end(struct solr_connection_post **_post)
        *_post = NULL;
 
        if (!post->failed) {
-               if (http_client_request_finish_payload(&post->http_req) <= 0 ||
+               if (http_client_request_finish_payload(&post->http_req) < 0 ||
                        conn->request_status < 0) {
                        ret = -1;
                }