Re: cvs commit: httpd-2.0/modules/experimental cache_util.c

2002-10-16 Thread Paul J. Reder

I spent a little time on Monday and Tuesday looking at this in the
debugger and it looks like the behavior I am seeing is probably
related to compiler optimizations. My testing showed that the fixes
you committed (64 bit atoi and conversion to use seconds) seem to
have fixed the random behavior I was seeing.

Thanks.

Brian Pane wrote:

 On Sat, 2002-10-12 at 20:26, Paul J. Reder wrote:
 
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.

 
 If I remember correctly, the ANSI arithmetic conversion rules
 should cause the 0 or -1 to be sign-extended to long long (or
 whatever other integral type apr_int64_t is typedef'ed to).
 
 Brian
 
 
 
 


-- 
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





Re: cvs commit: httpd-2.0/modules/experimental cache_util.c

2002-10-15 Thread Brian Pane

On Sat, 2002-10-12 at 20:26, Paul J. Reder wrote:
 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.

If I remember correctly, the ANSI arithmetic conversion rules
should cause the 0 or -1 to be sign-extended to long long (or
whatever other integral type apr_int64_t is typedef'ed to).

Brian





Re: cvs commit: httpd-2.0/modules/experimental cache_util.c

2002-10-13 Thread Paul J. Reder

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


[EMAIL PROTECTED] 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  ChangesPath
   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.c26 Aug 2002 16:41:56 -  1.18
   +++ cache_util.c12 Oct 2002 06:43:32 -  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