Modify

Ticket #1861 (closed Patches: fixed)

Opened 6 years ago

Last modified 5 years ago

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

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

Change History

Changed 6 years ago by ilya@…

comment:1 follow-up: ↓ 2 Changed 5 years ago by andysem

  • Status changed from new to closed
  • Resolution set to fixed

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 5 years ago by ilya.bobir@…

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

  • Cc ilya.bobir@… added
  • Status changed from closed to reopened
  • Version changed from Boost Development Trunk to Boost 1.38.0
  • Resolution fixed deleted

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 follow-up: ↓ 5 Changed 5 years ago by andysem

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 5 years ago by andysem

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 5 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 5 years ago by andysem

(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 5 years ago by andysem

  • Status changed from reopened to closed
  • Resolution set to fixed

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

View

Add a comment

Modify Ticket

Change Properties
<Author field>
Action
as closed
The resolution will be deleted. Next status will be 'reopened'
Author


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

 
Note: See TracTickets for help on using tickets.