Prefer "goto cleanup" over "do {... if (error) break; ... } while(0)"

construction for error handling/jump (as suggested by Ruediger).

Hence we can move backend->close = 1 (for mod_proxy_wstunnel) and
proxy_run_detach_backend() (for mod_proxy_http2) in the cleanup fallback.


git-svn-id: https://svn.apache.org/repos/asf/httpd/httpd/trunk@1729749 13f79535-47bb-0310-9956-ffa450edef68

2 files changed, 97 insertions, 107 deletions

diff --git a/modules/http2/mod_proxy_http2.c b/modules/http2/mod_proxy_http2.c
index d1eb963f2e..eb48c3dfc7 100644
--- a/modules/http2/mod_proxy_http2.c
+++ b/modules/http2/mod_proxy_http2.c
@@ -221,6 +221,7 @@ static int proxy_http2_handler(request_rec *r,
     apr_pool_t *p = r->pool;
     apr_uri_t *uri = apr_palloc(p, sizeof(*uri));
     const char *ssl_hostname = NULL;
+    conn_rec *backconn;
 
     /* find the scheme */
     if ((url[0] != 'h' && url[0] != 'H') || url[1] != '2') {
@@ -268,82 +269,74 @@ static int proxy_http2_handler(request_rec *r,
         ap_proxy_ssl_connection_cleanup(backend, r);
     }
 
-    do { /* while (0): break out */
-        conn_rec *backconn;
-        
-        /* Step One: Determine the URL to connect to (might be a proxy),
-         * initialize the backend accordingly and determine the server 
-         * port string we can expect in responses. */
-        if ((status = ap_proxy_determine_connection(p, r, conf, worker, backend,
-                                                    uri, &locurl, proxyname,
-                                                    proxyport, server_portstr,
-                                                    sizeof(server_portstr))) != OK) {
-            break;
-        }
-        
-        if (!ssl_hostname && backend->ssl_hostname) {
-            /* When reusing connections and finding sockets closed, the proxy
-             * framework loses the ssl_hostname setting. This is vital for us,
-             * so we save it once it is known. */
-            ssl_hostname = apr_pstrdup(r->pool, backend->ssl_hostname);
+    /* Step One: Determine the URL to connect to (might be a proxy),
+     * initialize the backend accordingly and determine the server 
+     * port string we can expect in responses. */
+    if ((status = ap_proxy_determine_connection(p, r, conf, worker, backend,
+                                                uri, &locurl, proxyname,
+                                                proxyport, server_portstr,
+                                                sizeof(server_portstr))) != OK) {
+        goto cleanup;
+    }
+    
+    if (!ssl_hostname && backend->ssl_hostname) {
+        /* When reusing connections and finding sockets closed, the proxy
+         * framework loses the ssl_hostname setting. This is vital for us,
+         * so we save it once it is known. */
+        ssl_hostname = apr_pstrdup(r->pool, backend->ssl_hostname);
+    }
+    
+    /* Step Two: Make the Connection (or check that an already existing
+     * socket is still usable). On success, we have a socket connected to
+     * backend->hostname. */
+    if (ap_proxy_connect_backend(proxy_function, backend, worker, r->server)) {
+        ap_log_rerror(APLOG_MARK, APLOG_ERR, 0, r, APLOGNO()
+                      "H2: failed to make connection to backend: %s",
+                      backend->hostname);
+        status = HTTP_SERVICE_UNAVAILABLE;
+        goto cleanup;
+    }
+    
+    /* Step Three: Create conn_rec for the socket we have open now. */
+    backconn = backend->connection;
+    if (!backconn) {
+        ap_log_rerror(APLOG_MARK, APLOG_DEBUG, status, r, APLOGNO()
+                      "setup new connection: is_ssl=%d %s %s %s, was %s", 
+                      backend->is_ssl, 
+                      backend->ssl_hostname, r->hostname, backend->hostname,
+                      ssl_hostname);
+        if ((status = ap_proxy_connection_create(proxy_function, backend,
+                                                 c, r->server)) != OK) {
+            goto cleanup;
         }
+        backconn = backend->connection;
         
-        /* Step Two: Make the Connection (or check that an already existing
-         * socket is still usable). On success, we have a socket connected to
-         * backend->hostname. */
-        if (ap_proxy_connect_backend(proxy_function, backend, worker, r->server)) {
-            ap_log_rerror(APLOG_MARK, APLOG_ERR, 0, r, APLOGNO()
-                          "H2: failed to make connection to backend: %s",
-                          backend->hostname);
-            status = HTTP_SERVICE_UNAVAILABLE;
-            break;
+        /*
+         * On SSL connections set a note on the connection what CN is
+         * requested, such that mod_ssl can check if it is requested to do
+         * so.
+         */
+        if (ssl_hostname) {
+            apr_table_setn(backend->connection->notes,
+                           "proxy-request-hostname", ssl_hostname);
         }
         
-        /* Step Three: Create conn_rec for the socket we have open now. */
-        backconn = backend->connection;
-        if (!backconn) {
-            ap_log_rerror(APLOG_MARK, APLOG_DEBUG, status, r, APLOGNO()
-                          "setup new connection: is_ssl=%d %s %s %s, was %s", 
-                          backend->is_ssl, 
-                          backend->ssl_hostname, r->hostname, backend->hostname,
-                          ssl_hostname);
-            if ((status = ap_proxy_connection_create(proxy_function, backend,
-                                                     c, r->server)) != OK) {
-                break;
-            }
-            backconn = backend->connection;
-            
-            /*
-             * On SSL connections set a note on the connection what CN is
-             * requested, such that mod_ssl can check if it is requested to do
-             * so.
-             */
-            if (ssl_hostname) {
-                apr_table_setn(backend->connection->notes,
-                               "proxy-request-hostname", ssl_hostname);
-            }
-            
-            if (backend->is_ssl) {
-                apr_table_setn(backend->connection->notes,
-                               "proxy-request-alpn-protos", "h2");
-            }
+        if (backend->is_ssl) {
+            apr_table_setn(backend->connection->notes,
+                           "proxy-request-alpn-protos", "h2");
         }
+    }
 
-        /* Step Four: Send the Request in a new HTTP/2 stream and
-         * loop until we got the response or encounter errors.
-         */
-        if ((status = proxy_http2_process_stream(p, url, r, &backend, worker,
-                                                 conf, server_portstr, 
-                                                 flushall)) != OK) {
-            ap_log_rerror(APLOG_MARK, APLOG_DEBUG, status, r, APLOGNO()
-                          "H2: failed to process request: %s",
-                          r->the_request);
-            backend->close = 1;
-            if (backend) {
-                proxy_run_detach_backend(r, backend);
-            }
-        }
-    } while (0);
+    /* Step Four: Send the Request in a new HTTP/2 stream and
+     * loop until we got the response or encounter errors.
+     */
+    if ((status = proxy_http2_process_stream(p, url, r, &backend, worker,
+                                             conf, server_portstr, 
+                                             flushall)) != OK) {
+        ap_log_rerror(APLOG_MARK, APLOG_DEBUG, status, r, APLOGNO()
+                      "H2: failed to process request: %s",
+                      r->the_request);
+    }
 
     /* clean up before return */
 cleanup:
@@ -351,6 +344,7 @@ cleanup:
         if (status != OK) {
             backend->close = 1;
         }
+        proxy_run_detach_backend(r, backend);
         proxy_http2_cleanup(proxy_function, r, backend);
     }
     ap_log_rerror(APLOG_MARK, APLOG_TRACE1, status, r, "leaving handler");
diff --git a/modules/proxy/mod_proxy_wstunnel.c b/modules/proxy/mod_proxy_wstunnel.c
index fb88a534cf..0ff50796bc 100644
--- a/modules/proxy/mod_proxy_wstunnel.c
+++ b/modules/proxy/mod_proxy_wstunnel.c
@@ -423,6 +423,7 @@ static int proxy_wstunnel_handler(request_rec *r, proxy_worker *worker,
     char *scheme;
     conn_rec *c = r->connection;
     apr_pool_t *p = r->pool;
+    char *locurl = url;
     apr_uri_t *uri;
     int is_ssl = 0;
 
@@ -449,53 +450,48 @@ static int proxy_wstunnel_handler(request_rec *r, proxy_worker *worker,
     ap_log_rerror(APLOG_MARK, APLOG_DEBUG, 0, r, APLOGNO(02451) "serving URL %s", url);
 
     /* create space for state information */
-    status = ap_proxy_acquire_connection(scheme, &backend, worker,
-                                         r->server);
+    status = ap_proxy_acquire_connection(scheme, &backend, worker, r->server);
     if (status != OK) {
-        if (backend) {
-            backend->close = 1;
-            ap_proxy_release_connection(scheme, backend, r->server);
-        }
-        return status;
+        goto cleanup;
     }
 
     backend->is_ssl = is_ssl;
     backend->close = 0;
 
-    do { /* while (0): break out */
-        char *locurl = url;
-        /* Step One: Determine Who To Connect To */
-        status = ap_proxy_determine_connection(p, r, conf, worker, backend,
-                                               uri, &locurl, proxyname, proxyport,
-                                               server_portstr,
-                                               sizeof(server_portstr));
-        if (status != OK)
-            break;
-
-        /* Step Two: Make the Connection */
-        if (ap_proxy_connect_backend(scheme, backend, worker, r->server)) {
-            ap_log_rerror(APLOG_MARK, APLOG_ERR, 0, r, APLOGNO(02452)
-                          "failed to make connection to backend: %s",
-                          backend->hostname);
-            status = HTTP_SERVICE_UNAVAILABLE;
-            break;
-        }
-        /* Step Three: Create conn_rec */
-        if (!backend->connection) {
-            if ((status = ap_proxy_connection_create(scheme, backend,
-                                                     c, r->server)) != OK)
-                break;
-        }
+    /* Step One: Determine Who To Connect To */
+    status = ap_proxy_determine_connection(p, r, conf, worker, backend,
+                                           uri, &locurl, proxyname, proxyport,
+                                           server_portstr,
+                                           sizeof(server_portstr));
+    if (status != OK) {
+        goto cleanup;
+    }
 
-        backend->close = 1; /* must be after ap_proxy_determine_connection */
+    /* Step Two: Make the Connection */
+    if (ap_proxy_connect_backend(scheme, backend, worker, r->server)) {
+        ap_log_rerror(APLOG_MARK, APLOG_ERR, 0, r, APLOGNO(02452)
+                      "failed to make connection to backend: %s",
+                      backend->hostname);
+        status = HTTP_SERVICE_UNAVAILABLE;
+        goto cleanup;
+    }
+
+    /* Step Three: Create conn_rec */
+    if (!backend->connection) {
+        status = ap_proxy_connection_create(scheme, backend, c, r->server);
+        if (status  != OK) {
+            goto cleanup;
+        }
+    }
 
-        /* Step Three: Process the Request */
-        status = proxy_wstunnel_request(p, r, backend, worker, conf, uri, locurl,
-                                      server_portstr, scheme);
-    } while (0);
+    /* Step Three: Process the Request */
+    status = proxy_wstunnel_request(p, r, backend, worker, conf, uri, locurl,
+                                  server_portstr, scheme);
 
+cleanup:
     /* Do not close the socket */
-    if (status != SUSPENDED) { 
+    if (backend && status != SUSPENDED) { 
+        backend->close = 1;
         ap_proxy_release_connection(scheme, backend, r->server);
     }
     return status;