You are viewing a plain text version of this content. The canonical link for it is here.
Posted to dev@httpd.apache.org by "Paul J. Reder" <re...@raleigh.ibm.com> on 2001/02/15 18:51:32 UTC
Re: cvs commit: httpd-2.0/modules/generators mod_status.c
Ryan,
rbb@apache.org wrote:
>
> rbb 01/02/14 16:01:34
>
> Modified: modules/generators mod_status.c
> Log:
> Remove some warnings from mod_status.
>
> Revision Changes Path
> 1.27 +7 -7 httpd-2.0/modules/generators/mod_status.c
>
> Index: mod_status.c
> ===================================================================
> RCS file: /home/cvs/httpd-2.0/modules/generators/mod_status.c,v
> retrieving revision 1.26
> retrieving revision 1.27
> diff -u -d -b -w -u -r1.26 -r1.27
> --- mod_status.c 2001/02/14 03:49:17 1.26
> +++ mod_status.c 2001/02/15 00:01:32 1.27
This patch causes core dumps on Linux. In particular the %lld change
causes ap_rprintf -> apr_vrprintf -> apr_vsnprintf -> apr_vformatter
%s processing (line 938) to dump core due to bad results from the
va_arg call (s is non-null garbage, causing strlen to dump).
Setting it back to %ld seems to work fine and doesn't seem to give me
warnings.
To reproduce this, all I have to do is start Apache (configed as
apache.org is) and try to browse server-status -- core dump!
> if (days)
> - ap_rprintf(r, " %ld day%s", days, days == 1 ? "" : "s");
> + ap_rprintf(r, " %lld day%s", days, days == 1 ? "" : "s");
> if (hrs)
> - ap_rprintf(r, " %ld hour%s", hrs, hrs == 1 ? "" : "s");
> + ap_rprintf(r, " %lld hour%s", hrs, hrs == 1 ? "" : "s");
> if (mins)
> - ap_rprintf(r, " %ld minute%s", mins, mins == 1 ? "" : "s");
> + ap_rprintf(r, " %lld minute%s", mins, mins == 1 ? "" : "s");
> if (secs)
> - ap_rprintf(r, " %ld second%s", secs, secs == 1 ? "" : "s");
> + ap_rprintf(r, " %lld second%s", secs, secs == 1 ? "" : "s");
> }--
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/generators mod_status.c
Posted by "William A. Rowe, Jr." <wr...@rowe-clan.net>.
From: "Greg Stein" <gs...@lyra.org>
Sent: Thursday, February 15, 2001 6:02 PM
> No need to be antagonistic... people will tend to agree with your view, you
> know... we are generally reasonable people...
True, and sorry if that offended anyone. I didn't like the direction I thought
things were going... I'm not only anti-cast (we mostly all agree there) but I'm
also an old C++ hack who doesn't believe in changing meanings either :-)
Re: cvs commit: httpd-2.0/modules/generators mod_status.c
Posted by Greg Stein <gs...@lyra.org>.
On Thu, Feb 15, 2001 at 02:27:56PM -0600, William A. Rowe, Jr. wrote:
>...
> I will _veto_ any abuse of apr_interval_time_t or apr_time_t in a manner that isn't
> consistent with the definitions you cited above.
No need to be antagonistic... people will tend to agree with your view, you
know... we are generally reasonable people...
--
Greg Stein, http://www.lyra.org/
Re: cvs commit: httpd-2.0/modules/generators mod_status.c
Posted by "William A. Rowe, Jr." <wr...@rowe-clan.net>.
> Guys, we have a time value that doesn't fit as either of these. Either we
> need ANOTHER time type, or we have done something wrong.
>
> Please do not tell me:
>
> apr_time_t - apr_time_t != (apr_time_t | apr_interval_time_t)
For jimminy christmas's sake, of course we do! I've been preaching that for six
friggin months. We need to delininate beween apr_interval_short_t and apr_interval_t.
The _short_t is lossy (accurate only to +/- 35 minutes). apr_interval_t should be
a signed 64 bit number. Why signed? Because you need to differentiate between
now - past and past - now which are distinctly different values, both valid.
[I was always partial to 'delta' over 'interval' myself, but that's a nit.]
Re: cvs commit: httpd-2.0/modules/generators mod_status.c
Posted by Ben Laurie <be...@algroup.co.uk>.
dean gaudet wrote:
>
> On Fri, 16 Feb 2001, Rodent of Unusual Size wrote:
>
> > Greg Stein wrote:
> > >
> > > The type was apr_uint32_t and the format was %ld. Those are
> > > compatible.
> >
> > Um, do not some platforms define a 'long int' as 64 bits?
>
> yup.
>
> if you look at the C99 standard you'll see that stdint.h defines macros
> for declaring constants of a particular size (INT8_C(), INT16_C(), ...),
> and macros expanding to the right size qualifiers for printf/scanf. (i
> forget what the latter are.)
>
> there was some effort to do this in APR ... i see in my, now many months
> old, 2.0 tree that there's macros such as APR_SSIZE_T_FMT, APR_SIZE_T_FMT,
> and APR_OFF_T_FMT.
Heh. That was me again :-)
> the correct fix is to start defining things such as APR_UINT32_T_FMT,
> APR_TIME_T_FMT, ...
Of course it is. No-one ever listens!
Cheers,
Ben.
--
http://www.apache-ssl.org/ben.html
"There is no limit to what a man can do or how far he can go if he
doesn't mind who gets the credit." - Robert Woodruff
ApacheCon 2001! http://ApacheCon.com/
Re: cvs commit: httpd-2.0/modules/generators mod_status.c
Posted by rb...@covalent.net.
On Sun, 25 Feb 2001, dean gaudet wrote:
> On Fri, 16 Feb 2001, Rodent of Unusual Size wrote:
>
> > Greg Stein wrote:
> > >
> > > The type was apr_uint32_t and the format was %ld. Those are
> > > compatible.
> >
> > Um, do not some platforms define a 'long int' as 64 bits?
>
> yup.
>
> if you look at the C99 standard you'll see that stdint.h defines macros
> for declaring constants of a particular size (INT8_C(), INT16_C(), ...),
> and macros expanding to the right size qualifiers for printf/scanf. (i
> forget what the latter are.)
>
> there was some effort to do this in APR ... i see in my, now many months
> old, 2.0 tree that there's macros such as APR_SSIZE_T_FMT, APR_SIZE_T_FMT,
> and APR_OFF_T_FMT.
>
> the correct fix is to start defining things such as APR_UINT32_T_FMT,
> APR_TIME_T_FMT, ...
That, and start to use the apr_int32_t, apr_int64, etc.
Ryan
_______________________________________________________________________________
Ryan Bloom rbb@apache.org
406 29th St.
San Francisco, CA 94131
-------------------------------------------------------------------------------
Re: cvs commit: httpd-2.0/modules/generators mod_status.c
Posted by dean gaudet <dg...@arctic.org>.
On Fri, 16 Feb 2001, Rodent of Unusual Size wrote:
> Greg Stein wrote:
> >
> > The type was apr_uint32_t and the format was %ld. Those are
> > compatible.
>
> Um, do not some platforms define a 'long int' as 64 bits?
yup.
if you look at the C99 standard you'll see that stdint.h defines macros
for declaring constants of a particular size (INT8_C(), INT16_C(), ...),
and macros expanding to the right size qualifiers for printf/scanf. (i
forget what the latter are.)
there was some effort to do this in APR ... i see in my, now many months
old, 2.0 tree that there's macros such as APR_SSIZE_T_FMT, APR_SIZE_T_FMT,
and APR_OFF_T_FMT.
the correct fix is to start defining things such as APR_UINT32_T_FMT,
APR_TIME_T_FMT, ...
-dean
Re: cvs commit: httpd-2.0/modules/generators mod_status.c
Posted by Rodent of Unusual Size <Ke...@Golux.Com>.
Greg Stein wrote:
>
> The type was apr_uint32_t and the format was %ld. Those are
> compatible.
Um, do not some platforms define a 'long int' as 64 bits?
--
#ken P-)}
Ken Coar <http://Golux.Com/coar/>
Apache Software Foundation <http://www.apache.org/>
"Apache Server for Dummies" <http://Apache-Server.Com/>
"Apache Server Unleashed" <http://ApacheUnleashed.Com/>
RE: cvs commit: httpd-2.0/modules/generators mod_status.c
Posted by Cliff Woolley <cl...@yahoo.com>.
---Jeff Trawick (trawick@bellsouth.net) wrote:
> Uhhh I don't really want to touch this discussion with a ten foot pole,
> but I don't think apr_uint32_t and %ld are compatible (portably).
>
> apr_uint32_t is always 32 bits (I hope). %ld is for a long int, which
> is sometimes 32 bits and sometimes 64 bits.
>
> What am I missing?
I don't think you're missing anything at all... in fact, I'd bet that you're
right on. The latest solution is to use just plain old 'int' for the local
variables that end up getting printed and plain old '%d' in the format
string. (Boy, I sure hope that works portably. sheesh)
The apr_uint32_t vs. apr_interval_time_t vs. something else discussion is
now limited just to the argument to the function, which never gets printed.
Cool?
--Cliff
Re: cvs commit: httpd-2.0/modules/generators mod_status.c
Posted by Jeff Trawick <tr...@bellsouth.net>.
Greg Stein <gs...@lyra.org> writes:
> The type was apr_uint32_t and the format was %ld. Those are compatible. I
> think signedness might have been an issue.
Uhhh I don't really want to touch this discussion with a ten foot pole,
but I don't think apr_uint32_t and %ld are compatible (portably).
apr_uint32_t is always 32 bits (I hope). %ld is for a long int, which
is sometimes 32 bits and sometimes 64 bits.
What am I missing?
--
Jeff Trawick | trawickj@bellsouth.net | PGP public key at web site:
http://www.geocities.com/SiliconValley/Park/9289/
Born in Roswell... married an alien...
Re: cvs commit: httpd-2.0/modules/generators mod_status.c
Posted by Greg Stein <gs...@lyra.org>.
On Thu, Feb 15, 2001 at 04:34:08PM -0800, rbb@covalent.net wrote:
> > > Hold on... You can't use either apr_interval_t or apr_interval_short_t for seconds.
> > >
> > > apr_interval_t is USEC, guarenteed to hold some +/- 292271 years.
> > >
> > > apr_interval_short_t is USEC, guarenteed to hold only +/- 35 minutes
> > >
> > > If you want simply seconds, store them in an apr_int_t.
> >
> > That is *exactly* what FirstBill did. He put them into an apr_uint32_t. Then
> > Ryan went and monkeyed with it :-)
> >
> > The question is why did Ryan have a problem with apr_uint32_t? The code sure
> > looked fine to start with.
>
> All I know is I had four warnings with the old code, one per
> line. Regardless, this is fixed now, commit coming soon.
You shouldn't have had warnings. That is my point. There was something
*else* going wrong. The commit has just spun the problem around until you
don't see the original; we don't know what the original problem was, or if
it still exists.
The type was apr_uint32_t and the format was %ld. Those are compatible. I
think signedness might have been an issue.
Cheers,
-g
--
Greg Stein, http://www.lyra.org/
Re: cvs commit: httpd-2.0/modules/generators mod_status.c
Posted by rb...@covalent.net.
> > Hold on... You can't use either apr_interval_t or apr_interval_short_t for seconds.
> >
> > apr_interval_t is USEC, guarenteed to hold some +/- 292271 years.
> >
> > apr_interval_short_t is USEC, guarenteed to hold only +/- 35 minutes
> >
> > If you want simply seconds, store them in an apr_int_t.
>
> That is *exactly* what FirstBill did. He put them into an apr_uint32_t. Then
> Ryan went and monkeyed with it :-)
>
> The question is why did Ryan have a problem with apr_uint32_t? The code sure
> looked fine to start with.
All I know is I had four warnings with the old code, one per
line. Regardless, this is fixed now, commit coming soon.
Ryan
_______________________________________________________________________________
Ryan Bloom rbb@apache.org
406 29th St.
San Francisco, CA 94131
-------------------------------------------------------------------------------
Re: cvs commit: httpd-2.0/modules/generators mod_status.c
Posted by Greg Stein <gs...@lyra.org>.
On Thu, Feb 15, 2001 at 05:09:38PM -0600, William A. Rowe, Jr. wrote:
>...
> Hold on... You can't use either apr_interval_t or apr_interval_short_t for seconds.
>
> apr_interval_t is USEC, guarenteed to hold some +/- 292271 years.
>
> apr_interval_short_t is USEC, guarenteed to hold only +/- 35 minutes
>
> If you want simply seconds, store them in an apr_int_t.
That is *exactly* what FirstBill did. He put them into an apr_uint32_t. Then
Ryan went and monkeyed with it :-)
The question is why did Ryan have a problem with apr_uint32_t? The code sure
looked fine to start with.
Cheers,
-g
--
Greg Stein, http://www.lyra.org/
Re: cvs commit: httpd-2.0/modules/generators mod_status.c
Posted by rb...@covalent.net.
On Thu, 15 Feb 2001, Cliff Woolley wrote:
>
> --- "William A. Rowe, Jr." <wr...@rowe-clan.net> wrote:
> > Hold on... You can't use either apr_interval_t or apr_interval_short_t for seconds.
> >
> > apr_interval_t is USEC, guarenteed to hold some +/- 292271 years.
> > apr_interval_short_t is USEC, guarenteed to hold only +/- 35 minutes
>
> Crap, I missed that part. Minor details. ;-]
>
> Okay, I guess this is the best plan, given the two types above: Use an apr_interval_t as
> the parameter to show_time (this is the difference represented in USEC, obviously). Do
> the division by APR_USEC_PER_SEC to get seconds within the show_time. Make each of the
> local variables a plain old 32-bit int.
>
> How does that sound?
That sounds correct. Patch will be committed soon with all three names
attached to it.
Ryan
_______________________________________________________________________________
Ryan Bloom rbb@apache.org
406 29th St.
San Francisco, CA 94131
-------------------------------------------------------------------------------
Re: cvs commit: httpd-2.0/modules/generators mod_status.c
Posted by Cliff Woolley <cl...@yahoo.com>.
--- "William A. Rowe, Jr." <wr...@rowe-clan.net> wrote:
> Hold on... You can't use either apr_interval_t or apr_interval_short_t for seconds.
>
> apr_interval_t is USEC, guarenteed to hold some +/- 292271 years.
> apr_interval_short_t is USEC, guarenteed to hold only +/- 35 minutes
Crap, I missed that part. Minor details. ;-]
Okay, I guess this is the best plan, given the two types above: Use an apr_interval_t as
the parameter to show_time (this is the difference represented in USEC, obviously). Do
the division by APR_USEC_PER_SEC to get seconds within the show_time. Make each of the
local variables a plain old 32-bit int.
How does that sound?
--Cliff
__________________________________________________
Do You Yahoo!?
Get personalized email addresses from Yahoo! Mail - only $35
a year! http://personal.mail.yahoo.com/
Re: cvs commit: httpd-2.0/modules/generators mod_status.c
Posted by "William A. Rowe, Jr." <wr...@rowe-clan.net>.
From: <rb...@covalent.net>
Sent: Thursday, February 15, 2001 4:59 PM
> On Thu, 15 Feb 2001, Cliff Woolley wrote:
> > --- rbb@covalent.net wrote:
> > > Guys, we have a time value that doesn't fit as either of these. Either we
> > > need ANOTHER time type, or we have done something wrong.
> >
> > We need another type. OtherBill's suggestion is fine by me.
>
> Same here.
>
> > > Please do not tell me:
> > > apr_time_t - apr_time_t != (apr_time_t | apr_interval_time_t)
> >
> > Of course it does:
> >
> > apr_time_t - apr_time_t == apr_interval_time_t
> >
> > BUT
> >
> > ((apr_time_t - apr_time_t)/APR_USEC_PER_SEC) != (apr_time_t | apr_interval_time_t)
> >
> > Hence the problem.
>
> Yeah, but we could fix that by just removing the /APR_USEC_PER_SEC in the
> generator function, and put it back in the show_times function.
>
> Oh well. Let's just add the new type and be done with it.
Hold on... You can't use either apr_interval_t or apr_interval_short_t for seconds.
apr_interval_t is USEC, guarenteed to hold some +/- 292271 years.
apr_interval_short_t is USEC, guarenteed to hold only +/- 35 minutes
If you want simply seconds, store them in an apr_int_t.
Re: cvs commit: httpd-2.0/modules/generators mod_status.c
Posted by rb...@covalent.net.
On Thu, 15 Feb 2001, Cliff Woolley wrote:
>
> --- rbb@covalent.net wrote:
> > Yeah, but we could fix that by just removing the /APR_USEC_PER_SEC in the
> > generator function, and put it back in the show_times function.
>
> That would fix the semantic problem, yes. But you'd limit yourself to 35 minutes of
> uptime that could be represented, creating a different problem. =-)
Yeah, I know.
>
> > Oh well. Let's just add the new type and be done with it.
>
> ++1.
I'll add it ASAP.
Ryan
Re: cvs commit: httpd-2.0/modules/generators mod_status.c
Posted by Cliff Woolley <cl...@yahoo.com>.
--- rbb@covalent.net wrote:
> Yeah, but we could fix that by just removing the /APR_USEC_PER_SEC in the
> generator function, and put it back in the show_times function.
That would fix the semantic problem, yes. But you'd limit yourself to 35 minutes of
uptime that could be represented, creating a different problem. =-)
> Oh well. Let's just add the new type and be done with it.
++1.
--Cliff
__________________________________________________
Do You Yahoo!?
Get personalized email addresses from Yahoo! Mail - only $35
a year! http://personal.mail.yahoo.com/
Re: cvs commit: httpd-2.0/modules/generators mod_status.c
Posted by rb...@covalent.net.
On Thu, 15 Feb 2001, Cliff Woolley wrote:
> --- rbb@covalent.net wrote:
> > Guys, we have a time value that doesn't fit as either of these. Either we
> > need ANOTHER time type, or we have done something wrong.
>
> We need another type. OtherBill's suggestion is fine by me.
Same here.
> > Please do not tell me:
> > apr_time_t - apr_time_t != (apr_time_t | apr_interval_time_t)
>
> Of course it does:
>
> apr_time_t - apr_time_t == apr_interval_time_t
>
> BUT
>
> ((apr_time_t - apr_time_t)/APR_USEC_PER_SEC) != (apr_time_t | apr_interval_time_t)
>
> Hence the problem.
Yeah, but we could fix that by just removing the /APR_USEC_PER_SEC in the
generator function, and put it back in the show_times function.
Oh well. Let's just add the new type and be done with it.
Ryan
_______________________________________________________________________________
Ryan Bloom rbb@apache.org
406 29th St.
San Francisco, CA 94131
-------------------------------------------------------------------------------
Re: cvs commit: httpd-2.0/modules/generators mod_status.c
Posted by Cliff Woolley <cl...@yahoo.com>.
--- rbb@covalent.net wrote:
> Guys, we have a time value that doesn't fit as either of these. Either we
> need ANOTHER time type, or we have done something wrong.
We need another type. OtherBill's suggestion is fine by me.
> Please do not tell me:
> apr_time_t - apr_time_t != (apr_time_t | apr_interval_time_t)
Of course it does:
apr_time_t - apr_time_t == apr_interval_time_t
BUT
((apr_time_t - apr_time_t)/APR_USEC_PER_SEC) != (apr_time_t | apr_interval_time_t)
Hence the problem.
--Cliff
__________________________________________________
Do You Yahoo!?
Get personalized email addresses from Yahoo! Mail - only $35
a year! http://personal.mail.yahoo.com/
Re: cvs commit: httpd-2.0/modules/generators mod_status.c
Posted by rb...@covalent.net.
> From: "Cliff Woolley" <cl...@yahoo.com>
> Sent: Thursday, February 15, 2001 1:36 PM
>
>
> > I guess I won't argue this too loudly if people agree that apr_interval_time_t's
> > definition is flexible)...
>
> It is _NOT_ flexible [your definitions are correct below].
>
> > While up_time does represent a value related to time, neither apr_time_t nor
> > apr_interval_time_t is strictly appropriate:
>
> I agree
>
> > apr_time_t is defined to be a value in microseconds since the epoch... this
> > usage meets neither criteria and is clearly not the best choice.
> >
> > apr_interval_time_t is defined to be a value *in microseconds* that is the
> > difference between two apr_time_t's... this usage doesn't meet the first
> > criterion in that it's not in microseconds; furthermore,
> > apr_interval_time_t's are limited to representing intervals of about 70
> > minutes or so given the number of microseconds that can be represented in 32
> > bits, meaning that apr_interval_time_t's are really only useful for
> > representing timeouts which are generally much less than 70 minutes. If we
> > choose to be flexible about whether an apr_interval_time_t is in
> > microseconds or seconds, then I guess this isn't a big deal... but I'll
> > leave that up to you all to argue amongst yourselves. =-)
>
> +/- 35 minutes or so, your 70 minutes is based on an unsigned, which this isn't.
>
> I will _veto_ any abuse of apr_interval_time_t or apr_time_t in a manner that isn't
> consistent with the definitions you cited above.
Guys, we have a time value that doesn't fit as either of these. Either we
need ANOTHER time type, or we have done something wrong.
Please do not tell me:
apr_time_t - apr_time_t != (apr_time_t | apr_interval_time_t)
Ryan
_______________________________________________________________________________
Ryan Bloom rbb@apache.org
406 29th St.
San Francisco, CA 94131
-------------------------------------------------------------------------------
Re: cvs commit: httpd-2.0/modules/generators mod_status.c
Posted by "William A. Rowe, Jr." <wr...@covalent.net>.
From: "Cliff Woolley" <cl...@yahoo.com>
Sent: Thursday, February 15, 2001 1:36 PM
> I guess I won't argue this too loudly if people agree that apr_interval_time_t's
> definition is flexible)...
It is _NOT_ flexible [your definitions are correct below].
> While up_time does represent a value related to time, neither apr_time_t nor
> apr_interval_time_t is strictly appropriate:
I agree
> apr_time_t is defined to be a value in microseconds since the epoch... this
> usage meets neither criteria and is clearly not the best choice.
>
> apr_interval_time_t is defined to be a value *in microseconds* that is the
> difference between two apr_time_t's... this usage doesn't meet the first
> criterion in that it's not in microseconds; furthermore,
> apr_interval_time_t's are limited to representing intervals of about 70
> minutes or so given the number of microseconds that can be represented in 32
> bits, meaning that apr_interval_time_t's are really only useful for
> representing timeouts which are generally much less than 70 minutes. If we
> choose to be flexible about whether an apr_interval_time_t is in
> microseconds or seconds, then I guess this isn't a big deal... but I'll
> leave that up to you all to argue amongst yourselves. =-)
+/- 35 minutes or so, your 70 minutes is based on an unsigned, which this isn't.
I will _veto_ any abuse of apr_interval_time_t or apr_time_t in a manner that isn't
consistent with the definitions you cited above.
RE: cvs commit: httpd-2.0/modules/generators mod_status.c
Posted by Cliff Woolley <cl...@yahoo.com>.
I said:
> How about doing it this way? Treat up_time as an apr_uint32_t number of
> seconds of uptime; days, hrs, mins, and secs are also
> apr_uint32_t's.
This lost something in the translation from when it was a patch to when I
was replying to Ryan's commit. What I ended up saying was identical to what
Bill's commit originally did; what I meant to say was that you can use
apr_uint32_t without using %ld or %lld, which is where the trouble comes in.
In other words, the only part I was questioning was the distinction between
apr_interval_time_t and apr_uint32_t, essentially just agreeing
with/defending Bill's original choice of apr_uint32_t.
--Cliff
RE: cvs commit: httpd-2.0/modules/generators mod_status.c
Posted by Cliff Woolley <cl...@yahoo.com>.
> -----Original Message-----
> This is a time. We may want to convert those to apr_interval_time_t's
> instead of apr_time_t's, but the up_time value (which is what we are
> talking about here), is without a doubt a time value.
How about doing it this way? Treat up_time as an apr_uint32_t number of
seconds of uptime; days, hrs, mins, and secs are also apr_uint32_t's. I see
Ryan just committed a patch to use apr_interval_time_t's, but I think we
have to go with apr_uint32_t if we're being strict (but I guess I won't
argue this too loudly if people agree that apr_interval_time_t's definition
is flexible)...
While up_time does represent a value related to time, neither apr_time_t nor
apr_interval_time_t is strictly appropriate:
apr_time_t is defined to be a value in microseconds since the epoch... this
usage meets neither criteria and is clearly not the best choice.
apr_interval_time_t is defined to be a value *in microseconds* that is the
difference between two apr_time_t's... this usage doesn't meet the first
criterion in that it's not in microseconds; furthermore,
apr_interval_time_t's are limited to representing intervals of about 70
minutes or so given the number of microseconds that can be represented in 32
bits, meaning that apr_interval_time_t's are really only useful for
representing timeouts which are generally much less than 70 minutes. If we
choose to be flexible about whether an apr_interval_time_t is in
microseconds or seconds, then I guess this isn't a big deal... but I'll
leave that up to you all to argue amongst yourselves. =-)
--Cliff
Re: cvs commit: httpd-2.0/modules/generators mod_status.c
Posted by rb...@covalent.net.
This is a time. We may want to convert those to apr_interval_time_t's
instead of apr_time_t's, but the up_time value (which is what we are
talking about here), is without a doubt a time value.
Ryan
On Thu, 15 Feb 2001, Bill Stoddard wrote:
> Another problem I have (conceptually) with your patch Ryan is that we are
> using apr_time_t to hold values that are -not- apr time. That is the reason I
> used the apr_uint32_t types which your removed (and which removed some compile
> time warnings for me on Windows). Variables defined as apr_time_t should not
> be used as generic 64 bit storage area.
>
> Bill
>
>
> ----- Original Message -----
> From: <rb...@covalent.net>
> To: <ne...@apache.org>
> Sent: Thursday, February 15, 2001 1:09 PM
> Subject: Re: cvs commit: httpd-2.0/modules/generators mod_status.c
>
>
> >
> > I am actually in the middle of tracing this down. The problem I am
> > having, is that apr_time_t is a 64 bit integer, so %lld is correct. I
> > will commit a fix in the next few minutes.
> >
> > Ryan
> >
> > On Thu, 15 Feb 2001, Paul J. Reder wrote:
> >
> > > Ryan,
> > >
> > > rbb@apache.org wrote:
> > > >
> > > > rbb 01/02/14 16:01:34
> > > >
> > > > Modified: modules/generators mod_status.c
> > > > Log:
> > > > Remove some warnings from mod_status.
> > > >
> > > > Revision Changes Path
> > > > 1.27 +7 -7 httpd-2.0/modules/generators/mod_status.c
> > > >
> > > > Index: mod_status.c
> > > > ===================================================================
> > > > RCS file: /home/cvs/httpd-2.0/modules/generators/mod_status.c,v
> > > > retrieving revision 1.26
> > > > retrieving revision 1.27
> > > > diff -u -d -b -w -u -r1.26 -r1.27
> > > > --- mod_status.c 2001/02/14 03:49:17 1.26
> > > > +++ mod_status.c 2001/02/15 00:01:32 1.27
> > >
> > > This patch causes core dumps on Linux. In particular the %lld change
> > > causes ap_rprintf -> apr_vrprintf -> apr_vsnprintf -> apr_vformatter
> > > %s processing (line 938) to dump core due to bad results from the
> > > va_arg call (s is non-null garbage, causing strlen to dump).
> > >
> > > Setting it back to %ld seems to work fine and doesn't seem to give me
> > > warnings.
> > >
> > > To reproduce this, all I have to do is start Apache (configed as
> > > apache.org is) and try to browse server-status -- core dump!
> > >
> > > > if (days)
> > > > - ap_rprintf(r, " %ld day%s", days, days == 1 ? "" : "s");
> > > > + ap_rprintf(r, " %lld day%s", days, days == 1 ? "" : "s");
> > > > if (hrs)
> > > > - ap_rprintf(r, " %ld hour%s", hrs, hrs == 1 ? "" : "s");
> > > > + ap_rprintf(r, " %lld hour%s", hrs, hrs == 1 ? "" : "s");
> > > > if (mins)
> > > > - ap_rprintf(r, " %ld minute%s", mins, mins == 1 ? "" : "s");
> > > > + ap_rprintf(r, " %lld minute%s", mins, mins == 1 ? "" : "s");
> > > > if (secs)
> > > > - ap_rprintf(r, " %ld second%s", secs, secs == 1 ? "" : "s");
> > > > + ap_rprintf(r, " %lld second%s", secs, secs == 1 ? "" : "s");
> > > > }--
> > > 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
> > >
> > >
> >
> >
> >
> ______________________________________________________________________________
> _
> > Ryan Bloom rbb@apache.org
> > 406 29th St.
> > San Francisco, CA 94131
> > ----------------------------------------------------------------------------
> ---
> >
>
>
_______________________________________________________________________________
Ryan Bloom rbb@apache.org
406 29th St.
San Francisco, CA 94131
-------------------------------------------------------------------------------
Re: cvs commit: httpd-2.0/modules/generators mod_status.c
Posted by rb...@covalent.net.
> > > This patch causes core dumps on Linux. In particular the %lld change
> > > causes ap_rprintf -> apr_vrprintf -> apr_vsnprintf -> apr_vformatter
> > > %s processing (line 938) to dump core due to bad results from the
> > > va_arg call (s is non-null garbage, causing strlen to dump).
> > >
> > > Setting it back to %ld seems to work fine and doesn't seem to give me
> > > warnings.
BTW, setting it to %ld gives bad output on my box.
Ryan
> > >
> > > To reproduce this, all I have to do is start Apache (configed as
> > > apache.org is) and try to browse server-status -- core dump!
> > >
> > > > if (days)
> > > > - ap_rprintf(r, " %ld day%s", days, days == 1 ? "" : "s");
> > > > + ap_rprintf(r, " %lld day%s", days, days == 1 ? "" : "s");
> > > > if (hrs)
> > > > - ap_rprintf(r, " %ld hour%s", hrs, hrs == 1 ? "" : "s");
> > > > + ap_rprintf(r, " %lld hour%s", hrs, hrs == 1 ? "" : "s");
> > > > if (mins)
> > > > - ap_rprintf(r, " %ld minute%s", mins, mins == 1 ? "" : "s");
> > > > + ap_rprintf(r, " %lld minute%s", mins, mins == 1 ? "" : "s");
> > > > if (secs)
> > > > - ap_rprintf(r, " %ld second%s", secs, secs == 1 ? "" : "s");
> > > > + ap_rprintf(r, " %lld second%s", secs, secs == 1 ? "" : "s");
> > > > }--
> > > 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
> > >
> > >
> >
> >
> >
> ______________________________________________________________________________
> _
> > Ryan Bloom rbb@apache.org
> > 406 29th St.
> > San Francisco, CA 94131
> > ----------------------------------------------------------------------------
> ---
> >
>
>
_______________________________________________________________________________
Ryan Bloom rbb@apache.org
406 29th St.
San Francisco, CA 94131
-------------------------------------------------------------------------------
Re: cvs commit: httpd-2.0/modules/generators mod_status.c
Posted by Bill Stoddard <bi...@wstoddard.com>.
Another problem I have (conceptually) with your patch Ryan is that we are
using apr_time_t to hold values that are -not- apr time. That is the reason I
used the apr_uint32_t types which your removed (and which removed some compile
time warnings for me on Windows). Variables defined as apr_time_t should not
be used as generic 64 bit storage area.
Bill
----- Original Message -----
From: <rb...@covalent.net>
To: <ne...@apache.org>
Sent: Thursday, February 15, 2001 1:09 PM
Subject: Re: cvs commit: httpd-2.0/modules/generators mod_status.c
>
> I am actually in the middle of tracing this down. The problem I am
> having, is that apr_time_t is a 64 bit integer, so %lld is correct. I
> will commit a fix in the next few minutes.
>
> Ryan
>
> On Thu, 15 Feb 2001, Paul J. Reder wrote:
>
> > Ryan,
> >
> > rbb@apache.org wrote:
> > >
> > > rbb 01/02/14 16:01:34
> > >
> > > Modified: modules/generators mod_status.c
> > > Log:
> > > Remove some warnings from mod_status.
> > >
> > > Revision Changes Path
> > > 1.27 +7 -7 httpd-2.0/modules/generators/mod_status.c
> > >
> > > Index: mod_status.c
> > > ===================================================================
> > > RCS file: /home/cvs/httpd-2.0/modules/generators/mod_status.c,v
> > > retrieving revision 1.26
> > > retrieving revision 1.27
> > > diff -u -d -b -w -u -r1.26 -r1.27
> > > --- mod_status.c 2001/02/14 03:49:17 1.26
> > > +++ mod_status.c 2001/02/15 00:01:32 1.27
> >
> > This patch causes core dumps on Linux. In particular the %lld change
> > causes ap_rprintf -> apr_vrprintf -> apr_vsnprintf -> apr_vformatter
> > %s processing (line 938) to dump core due to bad results from the
> > va_arg call (s is non-null garbage, causing strlen to dump).
> >
> > Setting it back to %ld seems to work fine and doesn't seem to give me
> > warnings.
> >
> > To reproduce this, all I have to do is start Apache (configed as
> > apache.org is) and try to browse server-status -- core dump!
> >
> > > if (days)
> > > - ap_rprintf(r, " %ld day%s", days, days == 1 ? "" : "s");
> > > + ap_rprintf(r, " %lld day%s", days, days == 1 ? "" : "s");
> > > if (hrs)
> > > - ap_rprintf(r, " %ld hour%s", hrs, hrs == 1 ? "" : "s");
> > > + ap_rprintf(r, " %lld hour%s", hrs, hrs == 1 ? "" : "s");
> > > if (mins)
> > > - ap_rprintf(r, " %ld minute%s", mins, mins == 1 ? "" : "s");
> > > + ap_rprintf(r, " %lld minute%s", mins, mins == 1 ? "" : "s");
> > > if (secs)
> > > - ap_rprintf(r, " %ld second%s", secs, secs == 1 ? "" : "s");
> > > + ap_rprintf(r, " %lld second%s", secs, secs == 1 ? "" : "s");
> > > }--
> > 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
> >
> >
>
>
>
______________________________________________________________________________
_
> Ryan Bloom rbb@apache.org
> 406 29th St.
> San Francisco, CA 94131
> ----------------------------------------------------------------------------
---
>
Re: cvs commit: httpd-2.0/modules/generators mod_status.c
Posted by rb...@covalent.net.
I am actually in the middle of tracing this down. The problem I am
having, is that apr_time_t is a 64 bit integer, so %lld is correct. I
will commit a fix in the next few minutes.
Ryan
On Thu, 15 Feb 2001, Paul J. Reder wrote:
> Ryan,
>
> rbb@apache.org wrote:
> >
> > rbb 01/02/14 16:01:34
> >
> > Modified: modules/generators mod_status.c
> > Log:
> > Remove some warnings from mod_status.
> >
> > Revision Changes Path
> > 1.27 +7 -7 httpd-2.0/modules/generators/mod_status.c
> >
> > Index: mod_status.c
> > ===================================================================
> > RCS file: /home/cvs/httpd-2.0/modules/generators/mod_status.c,v
> > retrieving revision 1.26
> > retrieving revision 1.27
> > diff -u -d -b -w -u -r1.26 -r1.27
> > --- mod_status.c 2001/02/14 03:49:17 1.26
> > +++ mod_status.c 2001/02/15 00:01:32 1.27
>
> This patch causes core dumps on Linux. In particular the %lld change
> causes ap_rprintf -> apr_vrprintf -> apr_vsnprintf -> apr_vformatter
> %s processing (line 938) to dump core due to bad results from the
> va_arg call (s is non-null garbage, causing strlen to dump).
>
> Setting it back to %ld seems to work fine and doesn't seem to give me
> warnings.
>
> To reproduce this, all I have to do is start Apache (configed as
> apache.org is) and try to browse server-status -- core dump!
>
> > if (days)
> > - ap_rprintf(r, " %ld day%s", days, days == 1 ? "" : "s");
> > + ap_rprintf(r, " %lld day%s", days, days == 1 ? "" : "s");
> > if (hrs)
> > - ap_rprintf(r, " %ld hour%s", hrs, hrs == 1 ? "" : "s");
> > + ap_rprintf(r, " %lld hour%s", hrs, hrs == 1 ? "" : "s");
> > if (mins)
> > - ap_rprintf(r, " %ld minute%s", mins, mins == 1 ? "" : "s");
> > + ap_rprintf(r, " %lld minute%s", mins, mins == 1 ? "" : "s");
> > if (secs)
> > - ap_rprintf(r, " %ld second%s", secs, secs == 1 ? "" : "s");
> > + ap_rprintf(r, " %lld second%s", secs, secs == 1 ? "" : "s");
> > }--
> 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
>
>
_______________________________________________________________________________
Ryan Bloom rbb@apache.org
406 29th St.
San Francisco, CA 94131
-------------------------------------------------------------------------------