BIND 10 #1470: address trivial fixes from francis' mail

[BIND 10 Development]

#1470: address trivial fixes from francis' mail
-------------------------------------+-------------------------------------
                   Reporter:  jelte  |                 Owner:  stephen
                       Type:         |                Status:  reviewing
  defect                             |             Milestone:
                   Priority:  major  |  Sprint-20111220
                  Component:         |            Resolution:
  Unclassified                       |             Sensitive:  0
                   Keywords:         |           Sub-Project:  DNS
            Defect Severity:  N/A    |  Estimated Difficulty:  5
Feature Depending on Ticket:  none   |           Total Hours:  0
        Add Hours to Ticket:  0      |
                  Internal?:  0      |
-------------------------------------+-------------------------------------
Changes (by jelte):

 * owner:  jelte => stephen


Comment:

 >
 > >  - replace shared_ptr by boost::shared_ptr in:
 <snip>
 > Implemented in delta from 0fd58479c441c0ce5584df6ab898594345e69ef5 to
 f1f4ce3e3014366d4916f924655c27761327c681
 >

 I've replaced a few more that I found, see commit
 502cc85e6d101d89210cf8a1a41f06ea8b2412dd

 >
 > >  - ***BUG*** in src/lib/cc/data.cc (first) removeIdentical() the
 > >   remove() can make the it iterator pointing to the end() so
 > >   the ++it is invalid
 > >   (I already signaled this bug, please fix it! BTW, what is the
 > >    escalation procedure?)
 > Fix implemented in the change from commit
 0213d987ac8b4fb30bc1aa1ee6bd67cdfde02ce0 to commit
 f18114000f75a0615ae97e36c54881754cc84be9
 >

 Do you know if this had a ticket too? (btw, should we add a comment to the
 added test that this is what it tests?)

 >
 > >  - in src/lib/dns/message.cc revisit the counts_ definition
 > >   (i.e., open a ticket for it so it will be done before the end
 > >    of the year)
 > Not done - there are any number of TODOs in the code that need
 addressing and this is just one of them.
 >

 Are there tickets for this yet?

 > >  - ***BUGS*** in src/lib/dns/rdata.cc there are two out of bound
 > >   accesses with empty vectors:
 > >    * line 115 where
 > >     buffer.readData(&data[0], rdata_len);
 > >    must be guard against rdata_len == 0
 > >    * line 245 where
 > >     if ((cmp = memcmp(&lhs.data_[0], &rhs.data_[0], len))
 > >    must be guard against len == 0
 > Implemented in delta from commit
 0a5f810dd86794befad38590f68d3725828379eb to commit
 f15d8831381af8a56c62e5e8a762166fcd053422.
 >
 > However, should these be assertions?  The Rdata class is apparently
 protected against zero length strings, so arguably if "len" is zero, there
 is a programming error.
 >

 I'd personally prefer exceptions that would result in a LOG_ERROR rather
 than asserts that result in abort(), but not sure whether we should check
 in the first place. Kinda depends on whether it would result in bad
 behaviour if it occurs (that we don't notice in our tests)

 For the rest the changes all look fine

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