httpd-dev mailing list archives

Site index · List index
Message view « Date » · « Thread »
Top « Date » · « Thread »
From "Paul J. Reder" <rede...@remulak.net>
Subject Re: cvs commit: httpd-2.0/modules/experimental cache_util.c
Date Sun, 13 Oct 2002 03:26:08 GMT
Okay, this takes care of item 4 from the list below. Thanks Brian, saves
me from having to do the commit. :)

What about the other 3? Should they be fixed by the change from
apr_time_t to apr_int64_t? Apr_time_t is really apr_int64_t under
the covers and I was seeing only the lower 32 bits being set when
the variables were assigned "0" and "-1". The value was correctly
set when it was assigned APR_DATE_BAD (which has an embedded cast)
so it seems that 1-3 still need to be done.

I haven't had a chance to re-run the tests through the debugger,
but I don't think the change from apr_time_t (apr_int64_t under
the covers) to apr_int64_t is going to remove the need for 1-3.

On Fri, 2002-10-11 at 16:04, Paul J. Reder wrote:
 > I have run into a problem where the cache code randomly decides that a
 > cached entry is stale or that the last modified date is some time in
 > the future. I tracked it back to the ap_cache_check_freshness code
 > which does a lot of checking of dates.
 >
 > Some of this date checking code compares and assigns uncast numeric values
 > (such as -1) and the output of atoi() to the apr_time_t values (which are
 > long longs). The debugger showed me that only the bottom half of the
 > apr_time_t was being updated/compared.
 >
 > I would like to fix the code in the following ways (if there are no
 >       objections):
 > 1) Replace the assignments/comparisons of 0 with APR_DATE_BAD
 > 2) Have someone create a "#define APR_DATE_UNASSIGNED ((apr_time_t)-1)"
 >         value in apr_date.h.
 > 3) Replace the assignments/comparisons of -1 with APR_DATE_UNASSIGNED
 > 4) Replace the atoi() calls with calls to apr_atoi64().

I can fix 1 and 3 (as shown in the patch I submitted for review) but I
need help from someone with APR commit authority to do number 2 (before
I can do 3).

Thanks,

Paul J. Reder


brianp@apache.org wrote:

> brianp      2002/10/11 23:43:32
> 
>   Modified:    modules/experimental cache_util.c
>   Log:
>   Fix a mismatch of data types: apr_time_t vs intervals measured
>   in seconds.  Also use 64-bit atoi conversions to avoid loss of
>   precision (thanks to Paul Reder for the atoi fix)
>   
>   Revision  Changes    Path
>   1.19      +9 -9      httpd-2.0/modules/experimental/cache_util.c
>   
>   Index: cache_util.c
>   ===================================================================
>   RCS file: /home/cvs/httpd-2.0/modules/experimental/cache_util.c,v
>   retrieving revision 1.18
>   retrieving revision 1.19
>   diff -u -r1.18 -r1.19
>   --- cache_util.c	26 Aug 2002 16:41:56 -0000	1.18
>   +++ cache_util.c	12 Oct 2002 06:43:32 -0000	1.19
>   @@ -138,7 +138,7 @@
>    
>    
>    /* do a HTTP/1.1 age calculation */
>   -CACHE_DECLARE(apr_time_t) ap_cache_current_age(cache_info *info, const apr_time_t
age_value)
>   +CACHE_DECLARE(apr_int64_t) ap_cache_current_age(cache_info *info, const apr_time_t
age_value)
>    {
>        apr_time_t apparent_age, corrected_received_age, response_delay, corrected_initial_age,
>               resident_time, current_age;
>   @@ -152,13 +152,13 @@
>        resident_time = apr_time_now() - info->response_time;
>        current_age = corrected_initial_age + resident_time;
>    
>   -    return (current_age);
>   +    return apr_time_sec(current_age);
>    }
>    
>    CACHE_DECLARE(int) ap_cache_check_freshness(cache_request_rec *cache, 
>                                                request_rec *r)
>    {
>   -    apr_time_t age, maxage_req, maxage_cresp, maxage, smaxage, maxstale, minfresh;
>   +    apr_int64_t age, maxage_req, maxage_cresp, maxage, smaxage, maxstale, minfresh;
>        const char *cc_cresp, *cc_req, *pragma_cresp;
>        const char *agestr = NULL;
>        char *val;
>   @@ -202,7 +202,7 @@
>        /* TODO: pragma_cresp not being used? */
>        pragma_cresp = apr_table_get(r->headers_out, "Pragma");  
>        if ((agestr = apr_table_get(r->headers_out, "Age"))) {
>   -        age_c = atoi(agestr);
>   +        age_c = apr_atoi64(agestr);
>        }
>    
>        /* calculate age of object */
>   @@ -210,19 +210,19 @@
>    
>        /* extract s-maxage */
>        if (cc_cresp && ap_cache_liststr(cc_cresp, "s-maxage", &val))
>   -        smaxage = atoi(val);
>   +        smaxage = apr_atoi64(val);
>        else
>            smaxage = -1;
>    
>        /* extract max-age from request */
>        if (cc_req && ap_cache_liststr(cc_req, "max-age", &val))
>   -        maxage_req = atoi(val);
>   +        maxage_req = apr_atoi64(val);
>        else
>            maxage_req = -1;
>    
>        /* extract max-age from response */
>        if (cc_cresp && ap_cache_liststr(cc_cresp, "max-age", &val))
>   -        maxage_cresp = atoi(val);
>   +        maxage_cresp = apr_atoi64(val);
>        else
>            maxage_cresp = -1;
>    
>   @@ -238,13 +238,13 @@
>    
>        /* extract max-stale */
>        if (cc_req && ap_cache_liststr(cc_req, "max-stale", &val))
>   -        maxstale = atoi(val);
>   +        maxstale = apr_atoi64(val);
>        else
>            maxstale = 0;
>    
>        /* extract min-fresh */
>        if (cc_req && ap_cache_liststr(cc_req, "min-fresh", &val))
>   -        minfresh = atoi(val);
>   +        minfresh = apr_atoi64(val);
>        else
>            minfresh = 0;
>    
>   
>   
>   
> 
> 
> 


-- 
Paul J. Reder
-----------------------------------------------------------
"The strength of the Constitution lies entirely in the determination of each
citizen to defend it.  Only if every single citizen feels duty bound to do
his share in this defense are the constitutional rights secure."
-- Albert Einstein



Mime
View raw message