httpd-dev mailing list archives

Site index · List index
Message view « Date » · « Thread »
Top « Date » · « Thread »
From Ruediger Pluem <rpl...@apache.org>
Subject Re: svn commit: r630974 - in /httpd/httpd/trunk/modules/ssl: ssl_engine_config.c ssl_private.h ssl_scache.c ssl_scache_dbm.c ssl_scache_dc.c ssl_scache_memcache.c ssl_scache_shmcb.c
Date Mon, 25 Feb 2008 20:49:55 GMT


On 02/25/2008 09:09 PM, jorton@apache.org wrote:
> Author: jorton
> Date: Mon Feb 25 12:09:38 2008
> New Revision: 630974
> 
> URL: http://svn.apache.org/viewvc?rev=630974&view=rev
> Log:
> Session cache interface redesign, Part 4:
> 
> Move provider-specific configuration handling down into the provider
> code.  Eliminate all use of SSLModConfigRec within provider code.
> 
> * modules/ssl/ssl_private.h (modssl_sesscache_provider): Add 'create'
>   function which creates and configures the cache provider, before
>   initialisation.  Change 'init' function to take the context pointer
>   as an input parameter, and reorder to be first.
> 
> * modules/ssl/ssl_scache.c (ssl_scache_init): Adjust accordingly.
> 
> * modules/ssl/ssl_scache_memcache.c (struct context): Add servers
>   field.
>   (ssl_scache_mc_create): New function.
>   (ssl_scache_mc_init): Use servers from context not SSLModConfigRec.
> 
> * modules/ssl/ssl_scache_dbm.c (struct context): Define.
>   (ssl_scache_dbm_create): New function.
>   (ssl_scache_dbm_init, ssl_scache_dbm_kill): Adjust to use filename
>   and pool from context.
>   (ssl_scache_dbm_store, ssl_scache_dbm_retrieve,
>   ssl_scache_dbm_status): Use filename from context.  Use context pool
>   for temp storage of the DBM object, and clear before use.
>   (ssl_scache_dbm_expire): Remove static tLast; use last_expiry from
>   context.  Use context pool for temp storage and clear before use.
> 
> * modules/ssl/ssl_scache_dc.c (struct context): Add target field.
>   (ssl_scache_dc_init, ssl_scache_dc_status): Use target from context.
> 
> * modules/ssl/ssl_scache_shmcb.c (struct context): Add data_file,
>   shm_size fields.
>   (ssl_scache_shmcb_create): New function; moved argument parsing
>   logic from ssl_cmd_SSLSessionCache
>   (ssl_scache_shmcb_init, ssl_scache_shmcb_status): Use config from
>   context.
> 
> * modules/ssl/ssl_engine_config.c (ssl_config_global_create): Remove
>   handling of old provider-specific fields.
>   (ssl_cmd_SSLSessionCache): Call provider ->create function to parse
>   the argument and create provider-specific context structure.
> 
> Modified:
>     httpd/httpd/trunk/modules/ssl/ssl_engine_config.c
>     httpd/httpd/trunk/modules/ssl/ssl_private.h
>     httpd/httpd/trunk/modules/ssl/ssl_scache.c
>     httpd/httpd/trunk/modules/ssl/ssl_scache_dbm.c
>     httpd/httpd/trunk/modules/ssl/ssl_scache_dc.c
>     httpd/httpd/trunk/modules/ssl/ssl_scache_memcache.c
>     httpd/httpd/trunk/modules/ssl/ssl_scache_shmcb.c
> 


>
> Modified: httpd/httpd/trunk/modules/ssl/ssl_scache_dbm.c
> URL: http://svn.apache.org/viewvc/httpd/httpd/trunk/modules/ssl/ssl_scache_dbm.c?rev=630974&r1=630973&r2=630974&view=diff
> ==============================================================================
> --- httpd/httpd/trunk/modules/ssl/ssl_scache_dbm.c (original)
> +++ httpd/httpd/trunk/modules/ssl/ssl_scache_dbm.c Mon Feb 25 12:09:38 2008
> @@ -26,19 +26,43 @@
>  
>  #include "ssl_private.h"
>  
> -static void ssl_scache_dbm_expire(void *context, server_rec *s);
> +/* Use of the context structure must be thread-safe after the initial
> + * create/init; callers must hold the mutex. */
> +struct context {
> +    const char *data_file;
> +    /* Pool must only be used with the mutex held. */
> +    apr_pool_t *pool;
> +    time_t last_expiry;
> +};
> +
> +static void ssl_scache_dbm_expire(struct context *ctx, server_rec *s);
>  
>  static void ssl_scache_dbm_remove(void *context, server_rec *s, UCHAR *id, int idlen,
>                                    apr_pool_t *p);
>  
> -static apr_status_t ssl_scache_dbm_init(server_rec *s, void **context, apr_pool_t *p)
> +static const char *ssl_scache_dbm_create(void **context, const char *arg, 
> +                                         apr_pool_t *tmp, apr_pool_t *p)
> +{
> +    struct context *ctx;
> +
> +    *context = ctx = apr_pcalloc(p, sizeof *ctx);

Hm. Don't we need to set ctx->pool to p or create a subpool of p and
assign it to ctx->pool?

> +
> +    ctx->data_file = ap_server_root_relative(p, arg);
> +    if (!ctx->data_file) {
> +        return apr_psprintf(tmp, "Invalid cache file path %s", arg);
> +    }
> +    
> +    return NULL;
> +}
> +
> +static apr_status_t ssl_scache_dbm_init(void *context, server_rec *s, apr_pool_t *p)
>  {
> -    SSLModConfigRec *mc = myModConfig(s);
> +    struct context *ctx = context;
>      apr_dbm_t *dbm;
>      apr_status_t rv;
>  
>      /* for the DBM we need the data file */
> -    if (mc->szSessionCacheDataFile == NULL) {
> +    if (ctx->data_file == NULL) {
>          ap_log_error(APLOG_MARK, APLOG_ERR, 0, s,
>                       "SSLSessionCache required");
>          return APR_EINVAL;

> @@ -274,12 +291,12 @@
>  
>      /* and delete it from the DBM file */
>      ssl_mutex_on(s);

Any particular reason why we do not clear ctx->pool here?

> -    if ((rv = apr_dbm_open(&dbm, mc->szSessionCacheDataFile,
> +    if ((rv = apr_dbm_open(&dbm, ctx->data_file,
>              APR_DBM_RWCREATE, SSL_DBM_FILE_MODE, p)) != APR_SUCCESS) {
>          ap_log_error(APLOG_MARK, APLOG_ERR, rv, s,
>                       "Cannot open SSLSessionCache DBM file `%s' for writing "
>                       "(delete)",
> -                     mc->szSessionCacheDataFile);
> +                     ctx->data_file);
>          ssl_mutex_off(s);
>          return;
>      }

> Modified: httpd/httpd/trunk/modules/ssl/ssl_scache_dc.c
> URL: http://svn.apache.org/viewvc/httpd/httpd/trunk/modules/ssl/ssl_scache_dc.c?rev=630974&r1=630973&r2=630974&view=diff
> ==============================================================================
> --- httpd/httpd/trunk/modules/ssl/ssl_scache_dc.c (original)
> +++ httpd/httpd/trunk/modules/ssl/ssl_scache_dc.c Mon Feb 25 12:09:38 2008
> @@ -49,22 +49,28 @@
>  */
>  
>  struct context {
> +    /* Configured target server: */
> +    const char *target;
> +    /* distcache client context: */
>      DC_CTX *dc;
>  };
>  
> -static apr_status_t ssl_scache_dc_init(server_rec *s, void **context, apr_pool_t *p)
> +static const char *ssl_scache_dc_create(void **context, const char *arg, 
> +                                        apr_pool_t *tmp, apr_pool_t *p)
>  {
> -    DC_CTX *dc;
> -    SSLModConfigRec *mc = myModConfig(s);
>      struct context *ctx;
>  
> -    /*
> -     * Create a session context
> -     */
> -    if (mc->szSessionCacheDataFile == NULL) {
> -        ap_log_error(APLOG_MARK, APLOG_ERR, 0, s, "SSLSessionCache required");
> -        return APR_EINVAL;
> -    }

Why don't we check any if arg == NULL as a replacement for the if statement above?

> +    ctx = *context = apr_palloc(p, sizeof *ctx);
> +    
> +    ctx->target = apr_pstrdup(p, arg);
> +
> +    return NULL;
> +}
> +
> +static apr_status_t ssl_scache_dc_init(void *context, server_rec *s, apr_pool_t *p)
> +{
> +    struct context *ctx = ctx;
> +
>  #if 0
>      /* If a "persistent connection" mode of operation is preferred, you *must*
>       * also use the PIDCHECK flag to ensure fork()'d processes don't interlace


> Modified: httpd/httpd/trunk/modules/ssl/ssl_scache_memcache.c
> URL: http://svn.apache.org/viewvc/httpd/httpd/trunk/modules/ssl/ssl_scache_memcache.c?rev=630974&r1=630973&r2=630974&view=diff
> ==============================================================================
> --- httpd/httpd/trunk/modules/ssl/ssl_scache_memcache.c (original)
> +++ httpd/httpd/trunk/modules/ssl/ssl_scache_memcache.c Mon Feb 25 12:09:38 2008
> @@ -68,10 +68,23 @@
>  #endif
>  
>  struct context {
> +    const char *servers;
>      apr_memcache_t *mc;
>  };
>  
> -static apr_status_t ssl_scache_mc_init(server_rec *s, void **context, apr_pool_t *p)
> +static const char *ssl_scache_mc_create(void **context, const char *arg, 
> +                                        apr_pool_t *tmp, apr_pool_t *p)
> +{
> +    struct context *ctx;
> +    
> +    *context = ctx = apr_palloc(p, sizeof *ctx);
> +
> +    ctx->servers = apr_pstrdup(p, arg);

Why don't we check any if arg == NULL as a replacement for the if statement below
checking mc->szSessionCacheDataFile == NULL


> +
> +    return NULL;
> +}
> +
> +static apr_status_t ssl_scache_mc_init(void *context, server_rec *s, apr_pool_t *p)
>  {
>      apr_status_t rv;
>      int thread_limit = 0;
> @@ -79,18 +92,12 @@
>      char *cache_config;
>      char *split;
>      char *tok;
> -    SSLModConfigRec *mc = myModConfig(s);
> -    struct context *ctx = apr_palloc(p, sizeof *ctx);
> +    struct context *ctx = context;
>  
>      ap_mpm_query(AP_MPMQ_HARD_LIMIT_THREADS, &thread_limit);
>  
> -    if (mc->szSessionCacheDataFile == NULL) {
> -        ap_log_error(APLOG_MARK, APLOG_ERR, 0, s, "SSLSessionCache required");
> -        return APR_EINVAL;
> -    }
> -
>      /* Find all the servers in the first run to get a total count */
> -    cache_config = apr_pstrdup(p, mc->szSessionCacheDataFile);
> +    cache_config = apr_pstrdup(p, ctx->servers);
>      split = apr_strtok(cache_config, ",", &tok);
>      while (split) {
>          nservers++;

Regards

RĂ¼diger

Mime
View raw message