Modify

Ticket #7868 (closed Bugs: fixed)

Opened 15 months ago

Last modified 7 months ago

chrono_io parses time incorrectly (1.53 and 1.52)

Reported by: Johan Lundberg <lundberj@…> Owned by: viboes
Milestone: Boost 1.55.0 Component: chrono
Version: Boost 1.54.0 Severity: Regression
Keywords: Cc:

Description

streaming the result of

boost::chrono::system_clock::now()

into a stringstream, and back to a

boost::chrono::system_clock::time_point

gives wrong results.

With boost 1.52 the error is exactly one second (when using the default formats)

With boost 1.53 beta (of today) the parsed times are semi-random around 1974.

With user-defined formats the error behaves a little differently:

In my example (attached) the time is correct up to the seconds, which are always parsed as 01 both with boost 1.52 and 1.53.

cheers, Johan Lundberg


Full example:

#define BOOST_CHRONO_VERSION 2
#define BOOST_CHRONO_PROVIDES_DATE_IO_FOR_SYSTEM_CLOCK_TIME_POINT 1

#include <sstream>
#include <iostream>
#include <boost/chrono/chrono_io.hpp>
#include <boost/chrono/floor.hpp>
#include <boost/chrono/round.hpp>
#include <boost/chrono/ceil.hpp>

int main()
{
  using namespace boost;
  using namespace boost::chrono;
  boost::chrono::system_clock::time_point atnow= boost::chrono::system_clock::now();
  {
      std::stringstream strm;
      std::stringstream strm2;
      // does not change anything: strm<<time_fmt(boost::chrono::timezone::utc);
      // does not change anything: strm2<<time_fmt(boost::chrono::timezone::utc);
      boost::chrono::system_clock::time_point atnow2;
      strm<<atnow<<std::endl;
      std::cout << "A:" << strm.str()<< std::endl;
      strm>>atnow2;      
      strm2<<atnow2<<std::endl;
      std::cout << "B:" << strm2.str()<< std::endl;
      
      // 1 sec wrong:
      std::cout << "diff:" << boost::chrono::duration_cast<microseconds>(atnow2 - atnow).count() <<std::endl;
      std::stringstream formatted;
      formatted << time_fmt(boost::chrono::timezone::utc, "%Y-%m-%d %H:%M:%S");
      formatted << "actual:"<< atnow <<std::endl;;
      formatted << "parsed:"<< atnow2 <<std::endl;;
      std::cout << formatted.str();
  }

  {
      std::stringstream strm;
      std::stringstream strm2;
      boost::chrono::system_clock::time_point atnow2;
      // the final second mark is always parsed as 01
      strm<<time_fmt(boost::chrono::timezone::utc, "%Y-%m-%d %H:%M:%S");
      strm2<<time_fmt(boost::chrono::timezone::utc, "%Y-%m-%d %H:%M:%S");
      strm<<atnow<<std::endl;
      std::cout << "actual:" << strm.str()<< std::endl;
      strm>>atnow2;
      strm2<<atnow2<<std::endl;
      // the final second mark is always parsed as 01
      std::cout << "parsed:" << strm2.str()<< std::endl;
  }
}

Details:

Compiled boost with gcc4.7.2 on Linux. I tried compiling boost both with and without cxxflags=-std=gnu++11, same result.

I also tried compiling my example with and without -std=c++11

Attachments

Change History

comment:1 Changed 15 months ago by viboes

  • Status changed from new to assigned

Hi,

I think that I have found the fix. Please could you try adding in boost/chrono/io/time_point_io.hpp the following missed initialization.

            tm.tm_sec=0;

before

#if defined BOOST_CHRONO_USES_INTERNAL_TIME_GET
            const detail::time_get<CharT>& dtg(tg);
            dtg.get(is, 0, is, err, &tm, pb, pe);
#else
            tg.get(is, 0, is, err, &tm, pb, pe);
#endif

comment:2 Changed 15 months ago by viboes

  • Milestone changed from To Be Determined to Boost 1.53.0

Here it is a patch to fix it.

svn diff boost/chrono 
Index: boost/chrono/config.hpp
===================================================================
--- boost/chrono/config.hpp	(revision 82527)
+++ boost/chrono/config.hpp	(working copy)
@@ -28,7 +28,7 @@
 #if ! defined BOOST_CHRONO_PROVIDES_DATE_IO_FOR_SYSTEM_CLOCK_TIME_POINT \
     && ! defined BOOST_CHRONO_DONT_PROVIDE_DATE_IO_FOR_SYSTEM_CLOCK_TIME_POINT
 
-# define BOOST_CHRONO_DONT_PROVIDE_DATE_IO_FOR_SYSTEM_CLOCK_TIME_POINT
+# define BOOST_CHRONO_PROVIDES_DATE_IO_FOR_SYSTEM_CLOCK_TIME_POINT
 
 #endif
 
Index: boost/chrono/io/time_point_io.hpp
===================================================================
--- boost/chrono/io/time_point_io.hpp	(revision 82527)
+++ boost/chrono/io/time_point_io.hpp	(working copy)
@@ -936,6 +936,7 @@
             { '%', 'Y', '-', '%', 'm', '-', '%', 'd', ' ', '%', 'H', ':', '%', 'M', ':' };
             pb = pattern;
             pe = pb + sizeof (pattern) / sizeof(CharT);
+            tm.tm_sec=0;
 #if defined BOOST_CHRONO_USES_INTERNAL_TIME_GET
             const detail::time_get<CharT>& dtg(tg);
             dtg.get(is, 0, is, err, &tm, pb, pe);

comment:3 Changed 15 months ago by viboes

Committed in trunk revision [82541].

comment:4 Changed 15 months ago by viboes

There is yet an issue with gmtime on Windows when the time_t parameter is < 0. I have added an internal implementation for this function in Changeset [82562].

Then I have made that system clock counts periods "since January 1, 1970, UTC" And activated the use of the internal gmtime function on windows in Changeset [82563].

This should fix the issue.

comment:5 Changed 15 months ago by viboes

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

Changeset [82612] Chrono: merge [82562][82663].

comment:6 Changed 10 months ago by lundberj@…

  • Status changed from closed to reopened
  • Version changed from Boost 1.52.0 to Boost 1.54.0
  • Resolution fixed deleted
  • Milestone changed from Boost 1.53.0 to Boost 1.54.0

I'm sorry to say that the bug still remains with beta 1.54 (beta0). Specifically the second part of the test code I posted, where the final seconds are always the same, and do not reflect the real time.

Other formats, such as "%Y-%m- %S %d %H:%M" seems to quietly fail.

In addition, documentation for this seems to be missing (at least I could not find it): What are the allowed markers (%S and friends) and what syntax must a format string have? (such as this one for date_time_io: http://www.boost.org/doc/libs/1_53_0/doc/html/date_time/date_time_io.html )

As and example, I wanted to find out the syntax for manually defining the default format, such as: 2013-06-30 20:01:15.584512 +0000 but I could not find out how to specify the fractional seconds or the time zone mark. Those are things a user should be able to find out.

comment:7 Changed 9 months ago by viboes

  • Milestone changed from Boost 1.54.0 to Boost 1.55.0

You are right. The ticket is not completely fixed. There was some comments in the time_point parsing input. The following patch should solve this particular issue, but I gues that I would need to complete the unit tets

Index: io/time_point_io.hpp
===================================================================
--- io/time_point_io.hpp	(revision 84889)
+++ io/time_point_io.hpp	(working copy)
@@ -255,9 +255,9 @@
 //                b = that_.get(b, e, iob, err, tm, fm, fm + sizeof(fm)/sizeof(fm[0]));
 //                }
 //                break;
-//            case 'S':
-//              that_.get_second(tm->tm_sec, b, e, err, ct);
-//                break;
+            case 'S':
+              get_second(tm->tm_sec, b, e, err, ct);
+                break;
 //            case 'T':
 //                {
 //                const char_type fm[] = {'%', 'H', ':', '%', 'M', ':', '%', 'S'};

comment:8 Changed 9 months ago by viboes

Committed revision [84967]. Committed revision [84973].

Last edited 9 months ago by viboes (previous) (diff)

comment:9 Changed 7 months ago by viboes

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

Changeset [85610]

comment:10 follow-up: ↓ 11 Changed 7 months ago by Johan Lundberg <lundberj@…>

Great! It also passes my tests.

A while back I reported compiler warnings on initializing the tm struct. The preferred solution at other places ( https://svn.boost.org/trac/boost/changeset/80797/branches/release/boost/date_time/date_facet.hpp) is

std::tm dtm; std::memset(&dtm, 0, sizeof(dtm));

That also initialize any allowed but unspecified members in addition to the ones required by the standard.

I still miss the fractional seconds part. Perhaps this is a feature-request, but that would make it possible to make full use of these facilities in a much wider range of applications. It would also allow to manually specify variants of the default format which does have fractional seconds, "1970-01-01 00:02:46.666667 +0000".

comment:11 in reply to: ↑ 10 Changed 7 months ago by viboes

Replying to Johan Lundberg <lundberj@…>:

Great! It also passes my tests.

A while back I reported compiler warnings on initializing the tm struct. The preferred solution at other places ( https://svn.boost.org/trac/boost/changeset/80797/branches/release/boost/date_time/date_facet.hpp) is

std::tm dtm; std::memset(&dtm, 0, sizeof(dtm));

That also initialize any allowed but unspecified members in addition to the ones required by the standard.

Please create a new bug ticket for that. I will fix it.

I still miss the fractional seconds part. Perhaps this is a feature-request, but that would make it possible to make full use of these facilities in a much wider range of applications. It would also allow to manually specify variants of the default format which does have fractional seconds, "1970-01-01 00:02:46.666667 +0000".

Please create a new feature request for any changes you need.

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.