Opened 14 years ago

Closed 5 years ago

#2819 closed Bugs (fixed)

boost::posix_time::hours, minutes, and seconds assume sizeof(long) == sizeof(int)

Reported by: pelee@… Owned by: James E. King, III
Milestone: Boost 1.67.0 Component: date_time
Version: Boost 1.37.0 Severity: Problem
Keywords: Cc:

Description

The constructors for boost::posix_time::hours, minutes, and seconds take arguments of type long, which is 64 bits on 64 bit LP64 environments like MacOS 10.5. This leads to warnings about possible truncation when these arguments are passed to the constructor for time_duration. Since time_duration is a template class that can be instantiated for different types, boost::sint32_t seems like a safer type to use in the non-templated constructors in boost::posix_time than a compiler-specific type like long.

  //! Allows expression of durations as an hour count
  /*! \ingroup time_basics
   */
  class hours : public time_duration
  {
  public:
	  explicit hours(boost::int32_t h) :
      time_duration(h,0,0)
    {}
  };

  //! Allows expression of durations as a minute count
  /*! \ingroup time_basics
   */
  class minutes : public time_duration
  {
  public:
	  explicit minutes(boost::int32_t m) :
      time_duration(0,m,0)
    {}
  };

  //! Allows expression of durations as a seconds count
  /*! \ingroup time_basics
   */
  class seconds : public time_duration
  {
  public:
	  explicit seconds(boost::int32_t s) :
      time_duration(0,0,s)
    {}
  };

Change History (6)

comment:1 by pelee@…, 14 years ago

Making this change also causes a 64-to-32 bit warning in xtime. The solution is to update a call to posix_time::seconds() to cast its argument to boost::int32_t instead of long.

    operator system_time() const
    {
        return boost::posix_time::from_time_t(0)+
			boost::posix_time::seconds(static_cast<boost::int32_t>(sec))+
#ifdef BOOST_DATE_TIME_HAS_NANOSECONDS
            boost::posix_time::nanoseconds(nsec);
#else
        boost::posix_time::microseconds((nsec+500)/1000);
#endif
    }

comment:2 by robert.stewart@…, 13 years ago

These same warnings occur for GCC 4.3.2 on 64b Linux.

Just casting to a smaller type is suspect as the caller can supply a value in the range of the larger type that overflows the smaller. The cast is acceptable if the range is checked, but it may be better to try to bring greater consistency to the integer types being used.

comment:3 by Rob Stewart <robert.stewart@…>, 12 years ago

I just ran afoul of this impedance mismatch yet again. The value fit long, but not int, the underlying type of time_duration::sec_type in this case. The truncation caused a large negative value to be treated as a sizable positive value silently creating a future rather than past time.

I think the proper solution is to change the classes to use the typedefs from the base class, giving the following:

  class hours : public time_duration
  {
  public:
    explicit hours(hour_type h) :
      time_duration(h,0,0)
    {}
  };

  class minutes : public time_duration
  {
  public:
    explicit minutes(min_type m) :
      time_duration(0,m,0)
    {}
  };

  class seconds : public time_duration
  {
  public:
    explicit seconds(sec_type s) :
      time_duration(0,0,s)
    {}
  };

Ensuring values fit within those types then becomes the caller's responsibility; DateTime won't silently truncate or otherwise alter them.

comment:4 by pankajuj@…, 11 years ago

Has anyone found a good fix for this issue? It looks like using the library for a 64 bit linux or mac platforms is not safe. Will the future version of boost have the fix?

comment:5 by James E. King, III, 5 years ago

Milestone: Boost 1.39.0Boost 1.67.0
Owner: changed from az_sw_dude to James E. King, III
Version: Boost Release BranchBoost 1.37.0

comment:6 by James E. King, III, 5 years ago

Resolution: fixed
Status: newclosed

Fix merged to master; resolved.

Note: See TracTickets for help on using tickets.