BIND 10 #2384: extend RRTTL "from text" constructor so it can accpet 1H, 2D, etc.

[BIND 10 Development]

#2384: extend RRTTL "from text" constructor so it can accpet 1H, 2D, etc.
-------------------------------------+-------------------------------------
                   Reporter:         |                 Owner:  jinmei
  jinmei                             |                Status:  reviewing
                       Type:  task   |             Milestone:
                   Priority:         |  Sprint-20121120
  medium                             |            Resolution:
                  Component:         |             Sensitive:  0
  libdns++                           |           Sub-Project:  DNS
                   Keywords:         |  Estimated Difficulty:  4
            Defect Severity:  N/A    |           Total Hours:  0
Feature Depending on Ticket:         |
  loadzone-ng                        |
        Add Hours to Ticket:  0      |
                  Internal?:  0      |
-------------------------------------+-------------------------------------

Comment (by jinmei):

 completing the review...

 '''other'''
 We need to update the documentation, explaining which format is
 accepted, maybe also noting some awkward cases like "30M1H" or even
 "1H2H".

 '''rrttl.cc'''

 - is static_cast needed?
 {{{#!cpp
         ttlval_ = static_cast<uint32_t>(val);
 }}}

 - is it okay to not check the range here?
 {{{#!cpp
             const int64_t value = (unit == pos) ? 1 :
                                   boost::lexical_cast<int64_t>(string(pos,
 unit));
 }}}
   For example, this implementation accepts "-1S1H".  Is it okay?
   How about this? "9223372036854775807S9223372036854775807S2S"
   (which would result in '0').  FYI: as far as I can see BIND 9 would
   reject it, although even BIND 9 doesn't seem to completely reject
   all overflow cases.

 - pretty minor, but I wonder whether we can avoid checking the
   condition of `pos != end` twice.  For example, if we eliminate the
   case of empty string, I guess the loop and condition could be:
 {{{#!cpp
     while (true) {
         ...
         if (pos == end) {
             break;
         }
         ++pos;
     }
 }}}

 '''rrttl_unittest'''

 - I'd test a case where the TTL text has more than two occurrences of
   the same unit, e.g., "10H10H"
 - not really for this ticket, but I suggest checking some boundary
   valid cases like RRTTL("0") and RRTTL("4294967295") to confirm they
   are actually accepted.  And, for that matter, these tests seem to
   belong to the wrong place:
 {{{#!cpp
 TEST_F(RRTTLTest, fromText) {
     EXPECT_EQ(0, ttl_0.getValue());
     EXPECT_EQ(3600, ttl_1h.getValue());
     EXPECT_EQ(86400, ttl_1d.getValue());
     EXPECT_EQ(0x12345678, ttl_32bit.getValue());
     EXPECT_EQ(0xffffffff, ttl_max.getValue());
 }}}
   because they have nothing to do with "from text" (again, not because
   of this ticket, though).

-- 
Ticket URL: <http://bind10.isc.org/ticket/2384#comment:6>
BIND 10 Development <http://bind10.isc.org>
BIND 10 Development