Author: brane Date: Tue Nov 15 12:34:08 2016 New Revision: 1769799 URL: http://svn.apache.org/viewvc?rev=1769799&view=rev Log: Make sure that serf_connection_create() initializes the config store.
* serf_private.h (serf__config_store_create_conn_config): Introduce an optional pool parameter for temporary allocations that, when provided, assures that this function will not return an error. Update the docstring. * src/config_store.c (serf__config_store_create_conn_config): Only create a temporary pool (and, hence, possibly return an error) if an optional pool was not provided. * src/outgoing.c (create_connection): New. Factor out most of the contents of serf_connection_create(). (serf_connection_create): Call create_connection(), then initialize the connection's config store. (serf_connection_create2): Call create_connection() instead of serf_connection_create(). Update the invocation of serf__config_store_create_conn_config(). * test/test_internal.c (test_config_store_per_connection_different_host, test_config_store_per_connection_same_host, test_config_store_remove_objects): Update all invocations of serf__config_store_create_conn_config(). Modified: serf/trunk/serf_private.h serf/trunk/src/config_store.c serf/trunk/src/outgoing.c serf/trunk/test/test_internal.c Modified: serf/trunk/serf_private.h URL: http://svn.apache.org/viewvc/serf/trunk/serf_private.h?rev=1769799&r1=1769798&r2=1769799&view=diff ============================================================================== --- serf/trunk/serf_private.h (original) +++ serf/trunk/serf_private.h Tue Nov 15 12:34:08 2016 @@ -334,11 +334,15 @@ apr_status_t serf__config_store_init(ser The host and connection entries will be created in the configuration store when not existing already. - The config object will be allocated in OUT_POOL. The config object's + The config object will be allocated in CONN's pool. The config object's lifecycle cannot extend beyond that of the serf context! + + If OPT_POOL is is not NULL, it will be used for temporary + allocations and the function will always return APR_SUCCESS. */ apr_status_t serf__config_store_create_conn_config(serf_connection_t *conn, - serf_config_t **config); + serf_config_t **config, + apr_pool_t *opt_pool); /* Same thing, but for incoming connections */ apr_status_t serf__config_store_create_client_config(serf_incoming_t *client, Modified: serf/trunk/src/config_store.c URL: http://svn.apache.org/viewvc/serf/trunk/src/config_store.c?rev=1769799&r1=1769798&r2=1769799&view=diff ============================================================================== --- serf/trunk/src/config_store.c (original) +++ serf/trunk/src/config_store.c Tue Nov 15 12:34:08 2016 @@ -236,21 +236,27 @@ apr_status_t serf__config_store_create_c } apr_status_t serf__config_store_create_conn_config(serf_connection_t *conn, - serf_config_t **config) + serf_config_t **config, + apr_pool_t *opt_pool) { serf__config_store_t *config_store = &conn->ctx->config_store; const char *host_key, *conn_key; serf__config_hdr_t *per_conn, *per_host; apr_pool_t *tmp_pool; - apr_status_t status; serf_config_t *cfg = apr_pcalloc(conn->pool, sizeof(serf_config_t)); cfg->ctx_pool = config_store->pool; cfg->allocator = config_store->allocator; cfg->per_context = config_store->global_per_context; - if ((status = apr_pool_create(&tmp_pool, cfg->ctx_pool)) != APR_SUCCESS) - return status; + if (!opt_pool) { + apr_status_t status = apr_pool_create(&tmp_pool, cfg->ctx_pool); + if (status != APR_SUCCESS) + return status; + } + else { + tmp_pool = opt_pool; + } /* Find the config values for this connection, create empty structure if needed */ @@ -279,7 +285,8 @@ apr_status_t serf__config_store_create_c } cfg->per_host = per_host; - apr_pool_destroy(tmp_pool); + if (tmp_pool != opt_pool) + apr_pool_destroy(tmp_pool); *config = cfg; Modified: serf/trunk/src/outgoing.c URL: http://svn.apache.org/viewvc/serf/trunk/src/outgoing.c?rev=1769799&r1=1769798&r2=1769799&view=diff ============================================================================== --- serf/trunk/src/outgoing.c (original) +++ serf/trunk/src/outgoing.c Tue Nov 15 12:34:08 2016 @@ -1216,7 +1216,7 @@ apr_status_t serf__process_connection(se return APR_SUCCESS; } -serf_connection_t *serf_connection_create( +static serf_connection_t *create_connection( serf_context_t *ctx, apr_sockaddr_t *address, serf_connection_setup_t setup, @@ -1265,6 +1265,8 @@ serf_connection_t *serf_connection_creat conn->done_reqs = conn->done_reqs_tail = 0; + conn->config = NULL; + /* Create a subpool for our connection. */ apr_pool_create(&conn->skt_pool, conn->pool); @@ -1278,6 +1280,23 @@ serf_connection_t *serf_connection_creat return conn; } +serf_connection_t *serf_connection_create( + serf_context_t *ctx, + apr_sockaddr_t *address, + serf_connection_setup_t setup, + void *setup_baton, + serf_connection_closed_t closed, + void *closed_baton, + apr_pool_t *pool) +{ + serf_connection_t *conn = create_connection(ctx, address, + setup, setup_baton, + closed, closed_baton, + pool); + serf__config_store_create_conn_config(conn, &conn->config, pool); + return conn; +} + apr_status_t serf_connection_create2( serf_connection_t **conn, serf_context_t *ctx, @@ -1308,8 +1327,8 @@ apr_status_t serf_connection_create2( return status; } - c = serf_connection_create(ctx, host_address, setup, setup_baton, - closed, closed_baton, pool); + c = create_connection(ctx, host_address, setup, setup_baton, + closed, closed_baton, pool); /* We're not interested in the path following the hostname. */ c->host_url = apr_uri_unparse(c->pool, @@ -1324,7 +1343,7 @@ apr_status_t serf_connection_create2( } /* Store the connection specific info in the configuration store */ - status = serf__config_store_create_conn_config(c, &config); + status = serf__config_store_create_conn_config(c, &config, NULL); if (status) return status; c->config = config; Modified: serf/trunk/test/test_internal.c URL: http://svn.apache.org/viewvc/serf/trunk/test/test_internal.c?rev=1769799&r1=1769798&r2=1769799&view=diff ============================================================================== --- serf/trunk/test/test_internal.c (original) +++ serf/trunk/test/test_internal.c Tue Nov 15 12:34:08 2016 @@ -119,13 +119,13 @@ static void test_config_store_per_connec /* Test 1: This should return a config object with per_context, per_host and per_connection hash_table's initialized. */ CuAssertIntEquals(tc, APR_SUCCESS, - serf__config_store_create_conn_config(conn1, &cfg1)); + serf__config_store_create_conn_config(conn1, &cfg1, NULL)); CuAssertPtrNotNull(tc, cfg1->per_context); CuAssertPtrNotNull(tc, cfg1->per_host); CuAssertPtrNotNull(tc, cfg1->per_conn); /* Get a config object for the other connection also. */ CuAssertIntEquals(tc, APR_SUCCESS, - serf__config_store_create_conn_config(conn2, &cfg2)); + serf__config_store_create_conn_config(conn2, &cfg2, NULL)); /* Test 2: Get a non-existing per connection key, value should be NULL */ CuAssertIntEquals(tc, APR_SUCCESS, @@ -180,13 +180,13 @@ static void test_config_store_per_connec /* Test 1: This should return a config object with per_context, per_host and per_connection hash_table's initialized. */ CuAssertIntEquals(tc, APR_SUCCESS, - serf__config_store_create_conn_config(conn1, &cfg1)); + serf__config_store_create_conn_config(conn1, &cfg1, NULL)); CuAssertPtrNotNull(tc, cfg1->per_context); CuAssertPtrNotNull(tc, cfg1->per_host); CuAssertPtrNotNull(tc, cfg1->per_conn); /* Get a config object for the other connection also. */ CuAssertIntEquals(tc, APR_SUCCESS, - serf__config_store_create_conn_config(conn2, &cfg2)); + serf__config_store_create_conn_config(conn2, &cfg2, NULL)); /* Test 2: Get a non-existing per connection key, value should be NULL */ CuAssertIntEquals(tc, APR_SUCCESS, @@ -277,7 +277,7 @@ static void test_config_store_remove_obj conn_closed, NULL, tb->pool); CuAssertIntEquals(tc, APR_SUCCESS, - serf__config_store_create_conn_config(conn, &cfg)); + serf__config_store_create_conn_config(conn, &cfg, NULL)); /* Add and remove a key per-context */ CuAssertIntEquals(tc, APR_SUCCESS,