You are viewing a plain text version of this content. The canonical link for it is here.
Posted to dev@httpd.apache.org by Joe Orton <jo...@redhat.com> on 2008/02/25 11:59:06 UTC

Re: svn commit: r630323 - 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

On Sat, Feb 23, 2008 at 12:25:40PM +0100, Ruediger Pluem wrote:
> On 02/22/2008 10:09 PM, jorton@apache.org wrote:
>> --- httpd/httpd/trunk/modules/ssl/ssl_scache.c (original)
>> +++ httpd/httpd/trunk/modules/ssl/ssl_scache.c Fri Feb 22 13:09:40 2008
>> @@ -40,6 +40,20 @@
>>  void ssl_scache_init(server_rec *s, apr_pool_t *p)
>>  {
>>      SSLModConfigRec *mc = myModConfig(s);
>> +    apr_status_t rv;
>> +
>> +    /* ### push this up into scache_init??? */
>> +    {
>> +        void *data;
>> +        const char *userdata_key = "ssl_scache_init";
>> +
>> +        apr_pool_userdata_get(&data, userdata_key, s->process->pool);
>> +        if (!data) {
>> +            apr_pool_userdata_set((const void *)1, userdata_key,
>> +                                  apr_pool_cleanup_null, s->process->pool);
>> +            return;
>
> Slightly confused now. This should prevent that we initialize twice correct?
> If yes, shouldn't the return be in the else branch (aka if apr_pool_userdata_get
> returns data we have been already here once)?

Ah, no, that's not quite the intent.  The intent of the code is only to 
ignore the first post_config run during startup.  It's necessary and 
correct for this code to initialize the cache for every subsequent 
invocation of the post_config hook; the configuration may be different 
each time, and pconf is used as the parent pool for all this stuff (also 
cleared each time).

I've updated the comment here in r630805 since it was stale anyway - 
does that make sense now?

joe

Re: svn commit: r630323 - 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

Posted by Joe Orton <jo...@redhat.com>.
On Mon, Feb 25, 2008 at 08:51:23PM +0100, Ruediger Pluem wrote:
> On 02/25/2008 11:59 AM, Joe Orton wrote:
>> Ah, no, that's not quite the intent.  The intent of the code is only to 
>> ignore the first post_config run during startup.  It's necessary and 
>> correct for this code to initialize the cache for every subsequent 
>> invocation of the post_config hook; the configuration may be different 
>> each time, and pconf is used as the parent pool for all this stuff (also 
>> cleared each time).
>>
>> I've updated the comment here in r630805 since it was stale anyway - does 
>> that make sense now?
>
> Ah. Thanks for the clarification. This makes now sense to me. So its 
> exactly the other way round I anticipated: Initialize every time 
> except for the very first time, correct?

Yup, exactly right.

joe

Re: svn commit: r630323 - 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

Posted by Ruediger Pluem <rp...@apache.org>.

On 02/25/2008 11:59 AM, Joe Orton wrote:
> On Sat, Feb 23, 2008 at 12:25:40PM +0100, Ruediger Pluem wrote:
>> On 02/22/2008 10:09 PM, jorton@apache.org wrote:
>>> --- httpd/httpd/trunk/modules/ssl/ssl_scache.c (original)
>>> +++ httpd/httpd/trunk/modules/ssl/ssl_scache.c Fri Feb 22 13:09:40 2008
>>> @@ -40,6 +40,20 @@
>>>  void ssl_scache_init(server_rec *s, apr_pool_t *p)
>>>  {
>>>      SSLModConfigRec *mc = myModConfig(s);
>>> +    apr_status_t rv;
>>> +
>>> +    /* ### push this up into scache_init??? */
>>> +    {
>>> +        void *data;
>>> +        const char *userdata_key = "ssl_scache_init";
>>> +
>>> +        apr_pool_userdata_get(&data, userdata_key, s->process->pool);
>>> +        if (!data) {
>>> +            apr_pool_userdata_set((const void *)1, userdata_key,
>>> +                                  apr_pool_cleanup_null, s->process->pool);
>>> +            return;
>> Slightly confused now. This should prevent that we initialize twice correct?
>> If yes, shouldn't the return be in the else branch (aka if apr_pool_userdata_get
>> returns data we have been already here once)?
> 
> Ah, no, that's not quite the intent.  The intent of the code is only to 
> ignore the first post_config run during startup.  It's necessary and 
> correct for this code to initialize the cache for every subsequent 
> invocation of the post_config hook; the configuration may be different 
> each time, and pconf is used as the parent pool for all this stuff (also 
> cleared each time).
> 
> I've updated the comment here in r630805 since it was stale anyway - 
> does that make sense now?

Ah. Thanks for the clarification. This makes now sense to me. So its exactly the
other way round I anticipated: Initialize every time except for the very first
time, correct?

Regards

RĂ¼diger


>