lib-http: http_client_request_unref() now always sets *req=NULL

This makes its behavior consistent with other APIs in Dovecot.

Also http_client_request_finish() no longer sets req=NULL, because all of
its callers already keep a reference. Instead added an assert to make sure
the reference is there.

diff --git a/src/lib-http/http-client-connection.c b/src/lib-http/http-client-connection.c
index 85fe67e789b688ea7366dff273f8ee8afe366a96..43c6d1f31c628591fe8ee6507f9d6d244912af3c 100644 (file)
--- a/src/lib-http/http-client-connection.c
+++ b/src/lib-http/http-client-connection.c
@@ -518,7 +518,7 @@ static void http_client_payload_destroyed(struct http_client_request *req)
        conn->incoming_payload = NULL;
        conn->pending_request = NULL;
        http_client_connection_ref(conn);
-       http_client_request_finish(&req);
+       http_client_request_finish(req);
 
        /* room for new requests */
        if (http_client_connection_is_ready(conn))
@@ -532,7 +532,6 @@ static void http_client_payload_destroyed(struct http_client_request *req)
        conn->to_input =
                timeout_add_short(0, http_client_payload_destroyed_timeout, conn);
 
-       i_assert(req != NULL);
        http_client_request_unref(&req);
        http_client_connection_unref(&conn);
 }
@@ -575,7 +574,7 @@ http_client_connection_return_response(struct http_client_request *req,
        if (!http_client_connection_unref(&req->conn)) {
                /* the callback managed to get this connection destroyed */
                if (!retrying)
-                       http_client_request_finish(&req);
+                       http_client_request_finish(req);
                http_client_request_unref(&req);
                return FALSE;
        }
@@ -611,7 +610,7 @@ http_client_connection_return_response(struct http_client_request *req,
                }
        } else {
                req->conn = NULL;
-               http_client_request_finish(&req);
+               http_client_request_finish(req);
                http_client_request_unref(&req);
        }
 
@@ -632,7 +631,7 @@ static void http_client_connection_input(struct connection *_conn)
                (struct http_client_connection *)_conn;
        struct http_response response;
        struct http_client_request *const *reqs;
-       struct http_client_request *req = NULL;
+       struct http_client_request *req = NULL, *req_ref;
        enum http_response_payload_type payload_type;
        unsigned int count;
        int finished = 0, ret;
@@ -799,8 +798,11 @@ static void http_client_connection_input(struct connection *_conn)
                /* remove request from queue */
                array_delete(&conn->request_wait_list, 0, 1);
                aborted = (req->state == HTTP_REQUEST_STATE_ABORTED);
-               i_assert(req->refcount > 1 || aborted);
-               http_client_request_unref(&req);
+               req_ref = req;
+               if (!http_client_request_unref(&req_ref)) {
+                       i_assert(aborted);
+                       req = NULL;
+               }
                
                conn->close_indicated = response.connection_close;
 

diff --git a/src/lib-http/http-client-private.h b/src/lib-http/http-client-private.h
index 474616c101f0a299e9ccef2b5d57311d0b4b9b23..f78011f6fa22413acfa24d8fde5b26a11da11980 100644 (file)
--- a/src/lib-http/http-client-private.h
+++ b/src/lib-http/http-client-private.h
@@ -271,7 +271,8 @@ struct http_client {
 int http_client_init_ssl_ctx(struct http_client *client, const char **error_r);
 
 void http_client_request_ref(struct http_client_request *req);
-void http_client_request_unref(struct http_client_request **_req);
+/* Returns FALSE if unrefing destroyed the request entirely */
+bool http_client_request_unref(struct http_client_request **_req);
 void http_client_request_destroy(struct http_client_request **_req);
 
 int http_client_request_delay_from_response(struct http_client_request *req,
@@ -297,7 +298,7 @@ void http_client_request_error(struct http_client_request *req,
        unsigned int status, const char *error);
 void http_client_request_redirect(struct http_client_request *req,
        unsigned int status, const char *location);
-void http_client_request_finish(struct http_client_request **_req);
+void http_client_request_finish(struct http_client_request *req);
 
 struct connection_list *http_client_connection_list_init(void);
 

diff --git a/src/lib-http/http-client-request.c b/src/lib-http/http-client-request.c
index c1bc49e53234d2a7e4cdb31ea0963794c969b1eb..2c985d7d13080c65f3abc9d087c7570b9cd87772 100644 (file)
--- a/src/lib-http/http-client-request.c
+++ b/src/lib-http/http-client-request.c
@@ -156,15 +156,17 @@ void http_client_request_ref(struct http_client_request *req)
        req->refcount++;
 }
 
-void http_client_request_unref(struct http_client_request **_req)
+bool http_client_request_unref(struct http_client_request **_req)
 {
        struct http_client_request *req = *_req;
        struct http_client *client = req->client;
 
        i_assert(req->refcount > 0);
 
+       *_req = NULL;
+
        if (--req->refcount > 0)
-               return;
+               return TRUE;
 
        http_client_request_debug(req, "Free (requests left=%d)",
                client->requests_count);
@@ -198,7 +200,7 @@ void http_client_request_unref(struct http_client_request **_req)
        if (req->headers != NULL)
                str_free(&req->headers);
        pool_unref(&req->pool);
-       *_req = NULL;
+       return FALSE;
 }
 
 void http_client_request_destroy(struct http_client_request **_req)
@@ -206,6 +208,8 @@ void http_client_request_destroy(struct http_client_request **_req)
        struct http_client_request *req = *_req;
        struct http_client *client = req->client;
 
+       *_req = NULL;
+
        http_client_request_debug(req, "Destroy (requests left=%d)",
                client->requests_count);
 
@@ -218,8 +222,7 @@ void http_client_request_destroy(struct http_client_request **_req)
                req->destroy_callback = NULL;
                callback(req->destroy_context);
        }
-
-       http_client_request_unref(_req);
+       http_client_request_unref(&req);
 }
 
 void http_client_request_set_port(struct http_client_request *req,
@@ -695,8 +698,7 @@ http_client_request_continue_payload(struct http_client_request **_req,
 
        /* callback may have messed with our pointer,
           so unref using local variable */     
-       http_client_request_unref(&req);
-       if (req == NULL)
+       if (!http_client_request_unref(&req))
                *_req = NULL;
 
        if (conn != NULL)
@@ -1022,12 +1024,14 @@ void http_client_request_error_delayed(struct http_client_request **_req)
 
        i_assert(req->state == HTTP_REQUEST_STATE_ABORTED);
 
+       *_req = NULL;
+
        i_assert(req->delayed_error != NULL && req->delayed_error_status != 0);
        http_client_request_send_error(req, req->delayed_error_status,
                                       req->delayed_error);
        if (req->queue != NULL)
                http_client_queue_drop_request(req->queue, req);
-       http_client_request_destroy(_req);
+       http_client_request_destroy(&req);
 }
 
 void http_client_request_error(struct http_client_request *req,
@@ -1060,6 +1064,8 @@ void http_client_request_abort(struct http_client_request **_req)
        struct http_client_request *req = *_req;
        bool sending = (req->state == HTTP_REQUEST_STATE_PAYLOAD_OUT);
 
+       *_req = NULL;
+
        if (req->state >= HTTP_REQUEST_STATE_FINISHED)
                return;
 
@@ -1074,16 +1080,15 @@ void http_client_request_abort(struct http_client_request **_req)
                http_client_queue_drop_request(req->queue, req);
        if (req->payload_wait && req->client->ioloop != NULL)
                io_loop_stop(req->client->ioloop);
-       http_client_request_destroy(_req);
+       http_client_request_destroy(&req);
 }
 
-void http_client_request_finish(struct http_client_request **_req)
+void http_client_request_finish(struct http_client_request *req)
 {
-       struct http_client_request *req = *_req;
-
        if (req->state >= HTTP_REQUEST_STATE_FINISHED)
                return;
 
+       i_assert(req->refcount > 1);
        http_client_request_debug(req, "Finished");
 
        req->callback = NULL;
@@ -1093,7 +1098,7 @@ void http_client_request_finish(struct http_client_request **_req)
                http_client_queue_drop_request(req->queue, req);
        if (req->payload_wait && req->client->ioloop != NULL)
                io_loop_stop(req->client->ioloop);
-       http_client_request_unref(_req);
+       http_client_request_unref(&req);
 }
 
 void http_client_request_redirect(struct http_client_request *req,