[Boost-bugs] [Boost C++ Libraries] #3606: One potential bug, and lots of warnings in DateTime

Subject: [Boost-bugs] [Boost C++ Libraries] #3606: One potential bug, and lots of warnings in DateTime
From: Boost C++ Libraries (noreply_at_[hidden])
Date: 2009-11-11 10:02:59

#3606: One potential bug, and lots of warnings in DateTime
--------------------------+-------------------------------------------------
 Reporter:  johnmaddock   |       Owner:  az_sw_dude
     Type:  Bugs          |      Status:  new       
Milestone:  Boost 1.42.0  |   Component:  date_time 
  Version:  Boost 1.40.0  |    Severity:  Problem   
 Keywords:                |  
--------------------------+-------------------------------------------------
 Boost.DateTime when compiled at high warning levels emits rather a lot of
 warnings, at least one of which seems to indicate a genuine potential bug
 and/or trap for the unwary.  In addition these warnings are seen by
 libraries like Boost.Thread which depend on DateTime, so the effect is not
 limited to just DateTime itself.

 I'm attaching a log of the build for msvc-9 (with /W4) and gcc-4.4.1 (with
 -Wall -Wextra -pedantic) showing the warnings produced.

 As to the potential bug, consider this warning:

 {{{
 d:\data\boost\trunk\boost/date_time/date_clock_device.hpp(41) : warning
 C4244: 'argument' : conversion from 'int' to 'unsigned short', possible
 loss of data
         d:\data\boost\trunk\boost/date_time/date_clock_device.hpp(36) :
 while compiling class template member function
 'boost::gregorian::date::ymd_type
 boost::date_time::day_clock<date_type>::local_day_ymd(void)'
         with
         [
             date_type=boost::gregorian::date
         ]
         gregorian\testdate_input_facet.cpp(257) : see reference to class
 template instantiation 'boost::date_time::day_clock<date_type>' being
 compiled
         with
         [
             date_type=boost::gregorian::date
         ]
 }}}

 The problem here is that the integer argument representing the
 day/month/year gets truncated to a short in the constructor of
 greg_day/greg_month/greg_year.  The problem here is manifold: first off I
 can't find it documented anywhere what type the argument to a
 day/month/year type should be (conceptually speaking) so it's hard to
 justify an explicit type cast in generic code like this.  More
 importantly, the implicit cast taking place here can circumvent the error
 checking inside the day/month/year type, consider:

 {{{
    int i = 0xf0001;
    boost::gregorian::greg_day d(i);
 }}}

 You might expect this to throw since the value i is out of range, but it
 doesn't because i gets truncated to the value 1 before the constructor is
 called.  Thus if these types are accidentally constructed with a garbage
 value (whether within the DateTime headers, or in user code), the
 construction can actually succeed :-(

 I think one potential solution is to decouple error-checking from storage
 type - for example if the constructor for greg_day was changed to a
 template:

 template <class Integer>
 greg_day(Integer day_of_month);

 and likewise for the underlying base type constructor, then we can error
 check the actual argument passed, and only then cast it to the storage
 type used.  As a bonus, is there any reason why the storage type is a
 short?  Could it be an unsigned char?  The latter would decrease the size
 of a day/month/year type from 8 to 4 bytes.

 Anyhow let me know if you need help fixing the other warnings - I stopped
 when I saw this one, because it didn't look like the usual spurious
 warning!

 Cheers, John.

-- 
Ticket URL: <https://svn.boost.org/trac/boost/ticket/3606>
Boost C++ Libraries <http://www.boost.org/>
Boost provides free peer-reviewed portable C++ source libraries.