You are viewing a plain text version of this content. The canonical link for it is here.
Posted to commits@subversion.apache.org by rh...@apache.org on 2010/08/05 09:12:49 UTC

svn commit: r982487 - in /subversion/trunk/subversion/libsvn_ra_serf: ra_serf.h util.c

Author: rhuijben
Date: Thu Aug  5 07:12:49 2010
New Revision: 982487

URL: http://svn.apache.org/viewvc?rev=982487&view=rev
Log:
Plug a few more possible serf error leaks by making setup_request() and
svn_ra_serf__setup_serf_req() capable of returning normal svn_error_t *
instances.

This is mostly preparation for a second patch which changes a common
callback type to allow returning errors.

* subversion/libsvn_ra_serf/ra_serf.h
  (svn_ra_serf__setup_serf_req): Update prototype.

* subversion/libsvn_ra_serf/util.c
  (svn_ra_serf__setup_serf_req): Allow returning errors and return errors
    for authorization initialization failures.
  (setup_request_cb): New function, moving most of setup_request into this
    function to allow easier error handling.
  (setup_request): Implement common error handling and call setup_request_cb.

Modified:
    subversion/trunk/subversion/libsvn_ra_serf/ra_serf.h
    subversion/trunk/subversion/libsvn_ra_serf/util.c

Modified: subversion/trunk/subversion/libsvn_ra_serf/ra_serf.h
URL: http://svn.apache.org/viewvc/subversion/trunk/subversion/libsvn_ra_serf/ra_serf.h?rev=982487&r1=982486&r2=982487&view=diff
==============================================================================
--- subversion/trunk/subversion/libsvn_ra_serf/ra_serf.h (original)
+++ subversion/trunk/subversion/libsvn_ra_serf/ra_serf.h Thu Aug  5 07:12:49 2010
@@ -408,7 +408,7 @@ svn_ra_serf__handle_client_cert_pw(void 
  *
  * If CONTENT_TYPE is not-NULL, it will be sent as the Content-Type header.
  */
-void
+svn_error_t *
 svn_ra_serf__setup_serf_req(serf_request_t *request,
                             serf_bucket_t **req_bkt, serf_bucket_t **hdrs_bkt,
                             svn_ra_serf__connection_t *conn,

Modified: subversion/trunk/subversion/libsvn_ra_serf/util.c
URL: http://svn.apache.org/viewvc/subversion/trunk/subversion/libsvn_ra_serf/util.c?rev=982487&r1=982486&r2=982487&view=diff
==============================================================================
--- subversion/trunk/subversion/libsvn_ra_serf/util.c (original)
+++ subversion/trunk/subversion/libsvn_ra_serf/util.c Thu Aug  5 07:12:49 2010
@@ -494,7 +494,7 @@ apr_status_t svn_ra_serf__handle_client_
     return APR_SUCCESS;
 }
 
-void
+svn_error_t *
 svn_ra_serf__setup_serf_req(serf_request_t *request,
                             serf_bucket_t **req_bkt,
                             serf_bucket_t **ret_hdrs_bkt,
@@ -527,13 +527,15 @@ svn_ra_serf__setup_serf_req(serf_request
 
   /* Setup server authorization headers */
   if (conn->session->auth_protocol)
-    conn->session->auth_protocol->setup_request_func(conn, method, url,
-                                                     hdrs_bkt);
+    SVN_ERR(conn->session->auth_protocol->setup_request_func(conn, method, url,
+                                                             hdrs_bkt));
 
   /* Setup proxy authorization headers */
   if (conn->session->proxy_auth_protocol)
-    conn->session->proxy_auth_protocol->setup_request_func(conn, method,
-                                                           url, hdrs_bkt);
+    SVN_ERR(conn->session->proxy_auth_protocol->setup_request_func(conn,
+                                                                   method,
+                                                                   url,
+                                                                   hdrs_bkt));
 
 #if ! SERF_VERSION_AT_LEAST(0, 4, 0)
   /* Set up SSL if we need to */
@@ -559,6 +561,8 @@ svn_ra_serf__setup_serf_req(serf_request
     {
       *ret_hdrs_bkt = hdrs_bkt;
     }
+
+  return SVN_NO_ERROR;
 }
 
 svn_error_t *
@@ -1457,24 +1461,22 @@ cleanup:
   return status;
 }
 
-/* Implements the serf_request_setup_t interface (which sets up both a
-   request and its response handler callback).  If the CTX->setup()
+/* svn_error_t * returning wrapper for setup_request.  If the CTX->setup()
    callback is non-NULL, invoke it to carry out the majority of the
    serf_request_setup_t implementation.  Otherwise, perform default
    setup, with special handling for HEAD requests, and finer-grained
    callbacks invoked (if non-NULL) to produce the request headers and
    body. */
-static apr_status_t
-setup_request(serf_request_t *request,
-              void *setup_baton,
-              serf_bucket_t **req_bkt,
-              serf_response_acceptor_t *acceptor,
-              void **acceptor_baton,
-              serf_response_handler_t *handler,
-              void **handler_baton,
-              apr_pool_t *pool)
+static svn_error_t *
+setup_request_cb(serf_request_t *request,
+                 svn_ra_serf__handler_t *ctx,
+                 serf_bucket_t **req_bkt,
+                 serf_response_acceptor_t *acceptor,
+                 void **acceptor_baton,
+                 serf_response_handler_t *handler,
+                 void **handler_baton,
+                 apr_pool_t *pool)
 {
-  svn_ra_serf__handler_t *ctx = setup_baton;
   serf_bucket_t *headers_bkt;
 
   *acceptor = svn_ra_serf__accept_response;
@@ -1484,17 +1486,11 @@ setup_request(serf_request_t *request,
     {
       svn_ra_serf__response_handler_t response_handler;
       void *response_baton;
-      svn_error_t *error;
 
-      error = ctx->setup(request, ctx->setup_baton, req_bkt,
-                          acceptor, acceptor_baton,
-                          &response_handler, &response_baton,
-                          pool);
-      if (error)
-        {
-          ctx->session->pending_error = error;
-          return error->apr_err;
-        }
+      SVN_ERR(ctx->setup(request, ctx->setup_baton, req_bkt,
+                         acceptor, acceptor_baton,
+                         &response_handler, &response_baton,
+                         pool));
 
       ctx->response_handler = response_handler;
       ctx->response_baton = response_baton;
@@ -1519,9 +1515,9 @@ setup_request(serf_request_t *request,
           body_bkt = NULL;
         }
 
-      svn_ra_serf__setup_serf_req(request, req_bkt, &headers_bkt, ctx->conn,
-                                  ctx->method, ctx->path,
-                                  body_bkt, ctx->body_type);
+      SVN_ERR(svn_ra_serf__setup_serf_req(request, req_bkt, &headers_bkt,
+                                          ctx->conn, ctx->method, ctx->path,
+                                          body_bkt, ctx->body_type));
 
       if (ctx->header_delegate)
         {
@@ -1535,6 +1531,40 @@ setup_request(serf_request_t *request,
   return APR_SUCCESS;
 }
 
+/* Implements the serf_request_setup_t interface (which sets up both a
+   request and its response handler callback). Handles errors for
+   setup_request_cb */
+static apr_status_t
+setup_request(serf_request_t *request,
+              void *setup_baton,
+              serf_bucket_t **req_bkt,
+              serf_response_acceptor_t *acceptor,
+              void **acceptor_baton,
+              serf_response_handler_t *handler,
+              void **handler_baton,
+              apr_pool_t *pool)
+{
+  svn_ra_serf__handler_t *ctx = setup_baton;
+  svn_error_t *err;
+
+  err = setup_request_cb(request, ctx,
+                         req_bkt,
+                         acceptor, acceptor_baton,
+                         handler, handler_baton,
+                         pool);
+
+  if (err)
+    {
+      ctx->session->pending_error 
+                = svn_error_compose_create(ctx->session->pending_error,
+                                           err);
+
+      return err->apr_err;
+    }
+
+  return APR_SUCCESS;
+}
+
 serf_request_t *
 svn_ra_serf__request_create(svn_ra_serf__handler_t *handler)
 {



Re: svn commit: r982487 - in /subversion/trunk/subversion/libsvn_ra_serf: ra_serf.h util.c

Posted by Kamesh Jayachandran <ka...@collab.net>.
Sorry to nit-pick.

> +static svn_error_t *
> +setup_request_cb(serf_request_t *request,
> +                 svn_ra_serf__handler_t *ctx,
> +                 serf_bucket_t **req_bkt,
> +                 serf_response_acceptor_t *acceptor,
> +                 void **acceptor_baton,
> +                 serf_response_handler_t *handler,
> +                 void **handler_baton,
> +                 apr_pool_t *pool)
>    

I guess current(r982487) 'setup_request' should be called 
'setup_request_cb' and 'setup_request_cb' should be called 
'setup_request_proper or do_setup_request'

With regards
Kamesh Jayachandran