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: ilya@… 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)

time_facet.hpp.patch (3.7 KB ) - added by ilya@… 14 years ago.
date_time_long_hours_patch2.patch (12.4 KB ) - added by ilya.bobir@… 14 years ago.

Download all attachments as: .zip

Change History (9)

by ilya@…, 14 years ago

Attachment: time_facet.hpp.patch added

comment:1 by Andrey Semashev, 14 years ago

Resolution: fixed
Status: newclosed

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.

by ilya.bobir@…, 14 years ago

in reply to:  1 comment:2 by ilya.bobir@…, 14 years ago

Cc: ilya.bobir@… added
Resolution: fixed
Status: closedreopened
Version: Boost Development TrunkBoost 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):

  1. The default time_duration format value is changed to %O:%M:%S%F instead of %H:%M:%S%F.
  2. A number of tests added that deal with time_duration values longer then 24 hours.
  3. Bug in the boost/date_time/format_date_parser.hpp:var_string_to_int() fixed.
  4. 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.

comment:3 by Andrey Semashev, 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 Andrey Semashev, 13 years ago

The fix for var_string_to_int merged into the release branch in revision 53618 (will be released in 1.40).

in reply to:  3 comment:5 by Ilya Bobir <ilya.bobir@…>, 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 Andrey Semashev, 13 years ago

(In [56506]) Refs #1861. Changed the default format for time durations to "%-%O:%M:%S%F" instead of "%-%H:%M:%S%F".

comment:7 by Andrey Semashev, 13 years ago

Resolution: fixed
Status: reopenedclosed

(In [56545]) Fixes #1861, #2213 merged from trunk.

Note: See TracTickets for help on using tickets.