Modify

Opened 10 years ago

Closed 8 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@… 10 years ago.
date_time_long_hours_patch2.patch (12.4 KB) - added by ilya.bobir@… 9 years ago.

Download all attachments as: .zip

Change History (9)

Changed 10 years ago by ilya@…

Attachment: time_facet.hpp.patch added

comment:1 Changed 9 years ago by Andrey Semashev

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.

Changed 9 years ago by ilya.bobir@…

comment:2 in reply to:  1 Changed 9 years ago by ilya.bobir@…

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 Changed 8 years ago by Andrey Semashev

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 Changed 8 years ago by Andrey Semashev

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

comment:5 in reply to:  3 Changed 8 years ago by Ilya Bobir <ilya.bobir@…>

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 Changed 8 years ago by Andrey Semashev

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

comment:7 Changed 8 years ago by Andrey Semashev

Resolution: fixed
Status: reopenedclosed

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

Modify Ticket

Change Properties
Set your email in Preferences
Action
as closed The owner will remain az_sw_dude.
The resolution will be deleted.

Add Comment


E-mail address and name can be saved in the Preferences.

 
Note: See TracTickets for help on using tickets.