changeset 19818:2cc7c59f1311

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.
author Timo Sirainen <timo.sirainen@dovecot.fi>
date Mon, 22 Feb 2016 20:47:37 +0200
parents fa23955f6028
children c1a959cba67a
files src/lib-http/http-client-request.c src/lib-http/http-client.h src/plugins/fts-solr/solr-connection.c
diffstat 3 files changed, 24 insertions(+), 4 deletions(-) [+]
line wrap: on
line diff
--- a/src/lib-http/http-client-request.c	Mon Feb 22 20:34:46 2016 +0200
+++ b/src/lib-http/http-client-request.c	Mon Feb 22 20:47:37 2016 +0200
@@ -711,14 +711,30 @@
 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)
--- a/src/lib-http/http-client.h	Mon Feb 22 20:34:46 2016 +0200
+++ b/src/lib-http/http-client.h	Mon Feb 22 20:47:37 2016 +0200
@@ -236,9 +236,13 @@
 
 /* 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,
--- a/src/plugins/fts-solr/solr-connection.c	Mon Feb 22 20:34:46 2016 +0200
+++ b/src/plugins/fts-solr/solr-connection.c	Mon Feb 22 20:47:37 2016 +0200
@@ -532,7 +532,7 @@
 	*_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;
 		}