|
|
Boost Testing : |
From: Jeff Garland (jeff_at_[hidden])
Date: 2005-06-04 18:18:11
On Thu, 2 Jun 2005 21:06:31 -0700, Martyn Lovell wrote
> Hi everyone,
>
> Thanks for your feedback on this issue. I wanted to clarify a couple
> of things about VC8's handling of time_t. Several issues have been
> discussed in this thread that are somewhat unrelated, so I wanted to
> be sure to explain how they related to each other.
>
> First, in VC8 we've widened time_t to a signed 64 bit value by
> default, to avoid the well-understood 2038 problem. We provide a
> #define that you can use to revert to a 32 bit time_t if you need to,
> but we default to 64 bit out of the box. We also provide _time32_t
> and _time64_t and families of associated functions for those who
> need to be specific about the size of their time_t value, for
> example for reading data from older file formats. I don't think this
> change is at all relevant to the problem you're seeing.
I agree -- I don't think time_t changes have anything to do with the problem
at hand. And actually, I'm glad to see an early fix for the 2038 issue
getting into the pipeline.
> Second, we've added some new safe APIs in cases where the shape of
> the old API was dangerous. For example, localtime used to return a
> pointer to a static struct tm,
Is this not easily avoidable by passing the second parameter to localtime as
the result?
> giving rise to the potential for
> reentrancy or other problems. localtime_s corrects this problem.
Hmm, seems like localtime_r from Posix:
http://www.opengroup.org/onlinepubs/009695399/functions/localtime.html
Or is the origin something else. I guess my bottom line is what's the
motivation behind the function rename? As I recall the signature of
localtime_s is the same as localtime? Why not just fix localtime? I'm not
finding the beta msdn link that I read earlier at the moment...
> Again, I don't think this change is related to your problem, since
> the new and old API have the same validation code.
Agree.
> Third, we added validation to all our APIs to help detect invalid values
> being passed to them and ensure that programs were only working with
> data in the ranges our code is designed to handle. It is this
> validation that is likely causing the problem here, because we
> consider negative time_t values to be invalid.
>
> Though the C standard does not explicitly declare negative time_t
> values as invalid (it says that time_t is implementation defined),
> it does use
> (time_t)(-1) as an invalid value to be returned from, for example,
> mktime(). And as others have noted, our implementation has
> historically not attempted to correctly treat all negative time_t
> values.
>
> Having said that, we've had feedback from a number of customers
> recently on this issue. It seems that that the _invalid_parameter
> calls on negative time_t values are causing portability and
> compatibility problems. Even though this validation was in place for
> our other alpha and beta builds, and has found real bugs in existing
> code, it seems that it has also recently caused several independent
> testers problems -- not just the boost community.
All interesting, but the code isn't directly using a negative time_t value.
Here's exactly what's happening:
1) boost::gregorian::date type is set to 1400,Jan,1
2) Output code (operator<<) is invoked.
3) Internally the output code wants to call do_put_tm for localization
4) To do this it constructs an std::tm from the boost::gregorian::date.
a) tm.year gets set to 1400
b) since for tm formatting functions tm.year zero == 1900, 1900 is
subtracted from tm.year
c) do_put_tm is called with tm.year == -500
So AFAIK there is no negative time_t involved at all. It is a negative year
in std::tm that is the issue. This code works fine on vc7.1 and all pretty
much all other compilers. It's new code in date-time for the 1.33 release
which is trying to play nicer with the existing standard library capabilities
for localization of I/O.
> I'm going to chat to a couple of people tomorrow to finalise this
> issue, but the likelihood is we'll reduce the validation on negative
> time_t in localtime (and a couple of other places) to have a similar
> impact to earlier versions of VC - negative times won't work right,
> but they won't cause an _invalid_parameter call. This has the
> disadvantage that we'll lose the ability to catch these kinds of
> bugs as easily, but should reduce the portability impact between VC
> and POSIX systems.
Hopefully the above clarifies our actual dependency.
> Thanks again for your feedback. Please feel free to contact me if
> you find any other issues like this. We appreciate the feedback, are
> listening, and will respond.
Thanks -- your help is much appreciated :-) Let us know if you need
additional information or we can be of other help.
Jeff