Opened 14 years ago
Closed 13 years ago
#1861 closed Patches (fixed)
time_duration type can not be correctly written or read of the period spans 24 hours of more (vc9 and probably others).
Reported by: | Owned by: | az_sw_dude | |
---|---|---|---|
Milestone: | To Be Determined | Component: | date_time |
Version: | Boost 1.38.0 | Severity: | Problem |
Keywords: | Cc: | ilya.bobir@… |
Description
Currently date_time library uses time_put::put (that delegates to strftime) in a non portable way passing it a tm structure with tm_hour field with values outside 0-23 range. See ISO 14882-2003 22.2.5.3.1 par. 1 and ISO 9899-1999 7.23.3.5 par. 3.
The issue was already discussed on the Boost mailing list: http://lists.boost.org/Archives/boost/2007/12/131541.php
This patch makes boost::date_time::time_facet::put(... , time_duration_type) output %H itself. As well as modifies boost::date_time::time_input_facet::get(... , time_duration_type) to input %H field as a variable length integer.
Attachments (2)
Change History (9)
by , 14 years ago
Attachment: | time_facet.hpp.patch added |
---|
follow-up: 2 comment:1 by , 14 years ago
Resolution: | → fixed |
---|---|
Status: | new → closed |
by , 14 years ago
Attachment: | date_time_long_hours_patch2.patch added |
---|
comment:2 by , 14 years ago
Cc: | added |
---|---|
Resolution: | fixed |
Status: | closed → reopened |
Version: | Boost Development Trunk → Boost 1.38.0 |
I think that the default format for the time_duration should be changed to contain %O instead of %H as time_duration objects can by default contain unrestricted hours value. This change affects time_duration input and output only.
I've wrote tests in order to catch bugs with time_period objects not been capable of processing values that are longer then 24 hours.
One of the tests showed a bug in the boost/date_time/format_date_parser.hpp:var_string_to_int(). The function was able to read after the input data end in some cases.
All of these are contained in the patch (date_time_long_hours_patch2.patch):
- The default time_duration format value is changed to %O:%M:%S%F instead of %H:%M:%S%F.
- A number of tests added that deal with time_duration values longer then 24 hours.
- Bug in the boost/date_time/format_date_parser.hpp:var_string_to_int() fixed.
- Documentation changes. Default format specifications for time_duration updated. And the %H should probably be documented along with %O.
I think that 1 and 2 are definitely a continuation of the original bug thus I decided to reopen it instead of creating a new one.
The diff is against the 1.38.0 but the relevant files are identical to those in trunk at the moment.
follow-up: 5 comment:3 by , 13 years ago
I've applied fix for var_string_to_int to trunk. When the tests pass I'll merge it to the release branch.
As for duration default format change, this is a breaking change. Although it has a valid point, I'm not sure the change is justified enough. This has to be discussed on the ML.
comment:4 by , 13 years ago
The fix for var_string_to_int merged into the release branch in revision 53618 (will be released in 1.40).
comment:5 by , 13 years ago
Replying to andysem:
As for duration default format change, this is a breaking change. Although it has a valid point, I'm not sure the change is justified enough. This has to be discussed on the ML.
It seems that no one cares about this change, except for Rutger ter Borg (http://lists.boost.org/Archives/boost/2009/06/153112.php) who says the current behavior is broken. The only person except you and me who also replied on the thread was Stewart, Robert but he was talking about unrelated things like error handling for IO streams in general (http://lists.boost.org/Archives/boost/2009/06/152121.php).
Do you think the fix can be applied now?
comment:6 by , 13 years ago
comment:7 by , 13 years ago
Resolution: | → fixed |
---|---|
Status: | reopened → closed |
The proposed fix breaks IO for ISO-formatted dates, so %H is still limited to be 2 chars long. The new %O placeholder was added in order to format even longer durations.