Modify

Opened 5 years ago

Closed 4 years ago

Last modified 4 years ago

#7868 closed Bugs (fixed)

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 (0)

Change History (11)

comment:1 Changed 5 years ago by viboes

Status: newassigned

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

Milestone: To Be DeterminedBoost 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 5 years ago by viboes

Committed in trunk revision [82541].

comment:4 Changed 5 years 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 5 years ago by viboes

Resolution: fixed
Status: assignedclosed

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

comment:6 Changed 4 years ago by lundberj@…

Milestone: Boost 1.53.0Boost 1.54.0
Resolution: fixed
Status: closedreopened
Version: Boost 1.52.0Boost 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 4 years ago by viboes

Milestone: Boost 1.54.0Boost 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 4 years ago by viboes

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

Last edited 4 years ago by viboes (previous) (diff)

comment:9 Changed 4 years ago by viboes

Resolution: fixed
Status: reopenedclosed

Changeset [85610]

comment:10 Changed 4 years 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 4 years 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.

Modify Ticket

Change Properties
Set your email in Preferences
Action
as closed The owner will remain viboes.
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.