Return-Path: Delivered-To: apmail-apr-dev-archive@apr.apache.org Received: (qmail 65383 invoked by uid 500); 11 Oct 2002 23:04:56 -0000 Mailing-List: contact dev-help@apr.apache.org; run by ezmlm Precedence: bulk List-Post: List-Help: List-Unsubscribe: List-Subscribe: Delivered-To: mailing list dev@apr.apache.org Received: (qmail 64929 invoked from network); 11 Oct 2002 23:04:53 -0000 Message-ID: <3DA75915.80306@remulak.net> Date: Fri, 11 Oct 2002 19:04:53 -0400 From: "Paul J. Reder" User-Agent: Mozilla/5.0 (X11; U; Linux i686; en-US; rv:0.9.2.1) Gecko/20010901 X-Accept-Language: en-us MIME-Version: 1.0 To: "Apache Dev." CC: "APR dev list." Subject: [Patch]: ap_cache_check_freshness 64 bit oddities Content-Type: text/plain; charset=us-ascii; format=flowed Content-Transfer-Encoding: 7bit X-Spam-Rating: daedalus.apache.org 1.6.2 0/1000/N 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(). Please see the attached patch. With no objections (and a litle help from someone who has APR commit authority) I would like to commit this this weekend. Thanks, -- 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 Index: httpd-2.0/modules/experimental/cache_util.c =================================================================== RCS file: /home/cvspublic/httpd-2.0/modules/experimental/cache_util.c,v retrieving revision 1.18 diff -u -r1.18 cache_util.c --- httpd-2.0/modules/experimental/cache_util.c 26 Aug 2002 16:41:56 -0000 1.18 +++ httpd-2.0/modules/experimental/cache_util.c 11 Oct 2002 22:56:14 -0000 @@ -162,7 +162,7 @@ const char *cc_cresp, *cc_req, *pragma_cresp; const char *agestr = NULL; char *val; - apr_time_t age_c = 0; + apr_time_t age_c = APR_DATE_BAD; cache_info *info = &(cache->handle->cache_obj->info); /* @@ -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,53 +210,53 @@ /* extract s-maxage */ if (cc_cresp && ap_cache_liststr(cc_cresp, "s-maxage", &val)) - smaxage = atoi(val); + smaxage = apr_atoi64(val); else - smaxage = -1; + smaxage = APR_DATE_UNASSIGNED; /* 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; + maxage_req = APR_DATE_UNASSIGNED; /* 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; + maxage_cresp = APR_DATE_UNASSIGNED; /* * if both maxage request and response, the smaller one takes priority */ - if (-1 == maxage_req) + if (APR_DATE_UNASSIGNED == maxage_req) maxage = maxage_cresp; - else if (-1 == maxage_cresp) + else if (APR_DATE_UNASSIGNED == maxage_cresp) maxage = maxage_req; else maxage = MIN(maxage_req, maxage_cresp); /* extract max-stale */ if (cc_req && ap_cache_liststr(cc_req, "max-stale", &val)) - maxstale = atoi(val); + maxstale = apr_atoi64(val); else - maxstale = 0; + maxstale = APR_DATE_BAD; /* extract min-fresh */ if (cc_req && ap_cache_liststr(cc_req, "min-fresh", &val)) - minfresh = atoi(val); + minfresh = apr_atoi64(val); else - minfresh = 0; + minfresh = APR_DATE_BAD; /* override maxstale if must-revalidate or proxy-revalidate */ - if (maxstale && ((cc_cresp && + if ((maxstale != APR_DATE_BAD) && ((cc_cresp && ap_cache_liststr(cc_cresp, "must-revalidate", NULL)) || (cc_cresp && ap_cache_liststr(cc_cresp, "proxy-revalidate", NULL)))) - maxstale = 0; + maxstale = APR_DATE_BAD; /* handle expiration */ - if ((-1 < smaxage && age < (smaxage - minfresh)) || - (-1 < maxage && age < (maxage + maxstale - minfresh)) || + if ((APR_DATE_UNASSIGNED < smaxage && age < (smaxage - minfresh)) || + (APR_DATE_UNASSIGNED < maxage && age < (maxage + maxstale - minfresh)) || (info->expire != APR_DATE_BAD && age < (info->expire - info->date + maxstale - minfresh))) { /* it's fresh darlings... */ /* set age header on response */ @@ -264,8 +264,8 @@ apr_psprintf(r->pool, "%lu", (unsigned long)age)); /* add warning if maxstale overrode freshness calculation */ - if (!((-1 < smaxage && age < smaxage) || - (-1 < maxage && age < maxage) || + if (!((APR_DATE_UNASSIGNED < smaxage && age < smaxage) || + (APR_DATE_UNASSIGNED < maxage && age < maxage) || (info->expire != APR_DATE_BAD && (info->expire - info->date) > age))) { /* make sure we don't stomp on a previous warning */ apr_table_merge(r->headers_out, "Warning", "110 Response is stale"); Index: httpd-2.0/srclib/apr-util/include/apr_date.h =================================================================== RCS file: /home/cvspublic/apr-util/include/apr_date.h,v retrieving revision 1.8 diff -u -r1.8 apr_date.h --- httpd-2.0/srclib/apr-util/include/apr_date.h 24 May 2002 02:35:02 -0000 1.8 +++ httpd-2.0/srclib/apr-util/include/apr_date.h 11 Oct 2002 22:56:16 -0000 @@ -83,6 +83,7 @@ #define APR_DATE_BAD ((apr_time_t)0) +#define APR_DATE_UNASSIGNED ((apr_time_t)-1) /** * Compare a string to a mask