You are viewing a plain text version of this content. The canonical link for it is here.
Posted to dev@httpd.apache.org by "William A. Rowe Jr." <wr...@apache.org> on 2010/06/23 17:54:08 UTC
Re: svn commit: r957235 - in /httpd/httpd/trunk: CHANGES modules/cache/mod_socache_dbm.c
modules/cache/mod_socache_dc.c modules/cache/mod_socache_shmcb.c
On 6/23/2010 10:04 AM, niq@apache.org wrote:
>
>
> Author: niq
> Date: Wed Jun 23 15:04:57 2010
> New Revision: 957235
>
> URL: http://svn.apache.org/viewvc?rev=957235&view=rev
> Log:
> Fix return values from socache modules when a key is not found in cache
>
> Modified:
> httpd/httpd/trunk/CHANGES
> httpd/httpd/trunk/modules/cache/mod_socache_dbm.c
> httpd/httpd/trunk/modules/cache/mod_socache_dc.c
> httpd/httpd/trunk/modules/cache/mod_socache_shmcb.c
>
> Modified: httpd/httpd/trunk/CHANGES
> URL: http://svn.apache.org/viewvc/httpd/httpd/trunk/CHANGES?rev=957235&r1=957234&r2=957235&view=diff
> ==============================================================================
> --- httpd/httpd/trunk/CHANGES [utf-8] (original)
> +++ httpd/httpd/trunk/CHANGES [utf-8] Wed Jun 23 15:04:57 2010
> @@ -2,6 +2,8 @@
>
> Changes with Apache 2.3.7
>
> + *) socache modules: return APR_NOTFOUND when a lookup is not found [Nick Kew]
> +
> *) mod_authn_cache: new module [Nick Kew]
>
> *) core: Try to proceed with authorization even if authentication failed.
>
> Modified: httpd/httpd/trunk/modules/cache/mod_socache_dbm.c
> URL: http://svn.apache.org/viewvc/httpd/httpd/trunk/modules/cache/mod_socache_dbm.c?rev=957235&r1=957234&r2=957235&view=diff
> ==============================================================================
> --- httpd/httpd/trunk/modules/cache/mod_socache_dbm.c (original)
> +++ httpd/httpd/trunk/modules/cache/mod_socache_dbm.c Wed Jun 23 15:04:57 2010
> @@ -291,7 +291,7 @@ static apr_status_t socache_dbm_retrieve
> rc = apr_dbm_fetch(dbm, dbmkey, &dbmval);
> if (rc != APR_SUCCESS) {
> apr_dbm_close(dbm);
> - return rc;
> + return APR_NOTFOUND;
The other two changes were fine, but why truncate an apr_status_t value?!?
Re: svn commit: r957235 - in /httpd/httpd/trunk: CHANGES modules/cache/mod_socache_dbm.c
modules/cache/mod_socache_dc.c modules/cache/mod_socache_shmcb.c
Posted by "William A. Rowe Jr." <wr...@rowe-clan.net>.
On 6/23/2010 4:33 PM, Nick Kew wrote:
>
> On 23 Jun 2010, at 16:54, William A. Rowe Jr. wrote:
>>>
>>> --- httpd/httpd/trunk/modules/cache/mod_socache_dbm.c (original)
>>> +++ httpd/httpd/trunk/modules/cache/mod_socache_dbm.c Wed Jun 23 15:04:57 2010
>>> @@ -291,7 +291,7 @@ static apr_status_t socache_dbm_retrieve
>>> rc = apr_dbm_fetch(dbm, dbmkey, &dbmval);
>>> if (rc != APR_SUCCESS) {
>>> apr_dbm_close(dbm);
>>> - return rc;
>>> + return APR_NOTFOUND;
>>
>> The other two changes were fine, but why truncate an apr_status_t value?!?
>
> Startingpoint was empirical testing: mod_authn_socache with dbm backend
> was getting incorrect return values.
>
> A look at the apr_dbm source shows uncertainty over mapping DBM errors
> to APR status, including blanket use of APR_EGENERAL. Didn't see an
> obvious fix.
Well, please don't break an API that works. Could you revert that bit and
we can continue to study what appropriate, specific codes should be NOTFOUND?
Maybe APR_EEXIST is the current error?
Re: svn commit: r957235 - in /httpd/httpd/trunk: CHANGES modules/cache/mod_socache_dbm.c modules/cache/mod_socache_dc.c modules/cache/mod_socache_shmcb.c
Posted by Nick Kew <ni...@webthing.com>.
On 23 Jun 2010, at 16:54, William A. Rowe Jr. wrote:
>>
>> --- httpd/httpd/trunk/modules/cache/mod_socache_dbm.c (original)
>> +++ httpd/httpd/trunk/modules/cache/mod_socache_dbm.c Wed Jun 23 15:04:57 2010
>> @@ -291,7 +291,7 @@ static apr_status_t socache_dbm_retrieve
>> rc = apr_dbm_fetch(dbm, dbmkey, &dbmval);
>> if (rc != APR_SUCCESS) {
>> apr_dbm_close(dbm);
>> - return rc;
>> + return APR_NOTFOUND;
>
> The other two changes were fine, but why truncate an apr_status_t value?!?
Startingpoint was empirical testing: mod_authn_socache with dbm backend
was getting incorrect return values.
A look at the apr_dbm source shows uncertainty over mapping DBM errors
to APR status, including blanket use of APR_EGENERAL. Didn't see an
obvious fix.
--
Nick Kew