BIND 10 #2124: RFC 6594 for SSHFP

[BIND 10 Development]

#2124: RFC 6594 for SSHFP
-------------------------------------+-------------------------------------
                   Reporter:         |                 Owner:  muks
  vorner                             |                Status:  reviewing
                       Type:         |             Milestone:
  defect                             |  Sprint-20120731
                   Priority:         |            Resolution:
  medium                             |             Sensitive:  0
                  Component:         |           Sub-Project:  DNS
  libdns++                           |  Estimated Difficulty:  2
                   Keywords:         |           Total Hours:  0
            Defect Severity:  N/A    |
Feature Depending on Ticket:         |
        Add Hours to Ticket:  0      |
                  Internal?:  0      |
-------------------------------------+-------------------------------------

Comment (by muks):

 Replying to [comment:24 jinmei]:
 > I have one last thing to discuss.  (Unlike the "from text" case) I
 > wouldn't catch buffer exception in the from wire constructor at this
 > level:
 > {{{#!cpp
 >     try {
 > ...
 >     } catch (const isc::util::InvalidBufferPosition& e) {
 >         isc_throw(InvalidRdataLength,
 >                   "SSHFP record shorter than RDATA len: " << e.what());
 >     }
 > }}}
 > because in the case of from wire we normally use try-catch for a
 > higher level like "Message::fromWire", and since from wire
 > construction is relatively performance sensitive, and depending on the
 > implementation of the exception mechanism simply doing try-catch can
 > be expensive, and since (as far as I understand it) we don't catch
 > these errors within RDATA implementations for other types.

 Done. :)

 > And I have one minor suggestion: in SSHFP::compare() the condition
 > could be a bit simplified:
 > {{{#!cpp
 >     if (cmplen > 0) {
 >         const int cmp = memcmp(&fingerprint_[0],
 &other_sshfp.fingerprint_[0],
 >                                cmplen);
 >         if (cmp != 0) {
 >             return (cmp);
 >         }
 >     }
 >     return ((this_len == other_len)
 >             ? 0 : (this_len < other_len) ? -1 : 1);
 > }}}
 > then we can reduce the number of the ugly nested ?:.  In cases like
 > this I'd even avoid using ?: completely (I personally often use ?: if
 > by doing so we can make a variable const, but in this case there's no
 > such advantage).

 Done. :)

 >
 > But all of these are pretty minor.  I'd leave it to you.
 >
 > Finally, I've made a few editorial/style changes.  Some may need
 > discussions but to safe time I've directly committed these.  Please
 > check.

 The `.size()` to `.empty()` change looks fine, and I agree.

 > And really finally: I think we should be able to use the generic .spec
 > file with the generator script for the added test data.  Then we
 > wouldn't have the style issues (basically copy-paste problems) I fixed
 > in my commits.  But we've already spent too much time for this
 > supposedly simple ticket, so I don't suggest changing them at this
 > stage.

 I'll leave them as raw wiredata, as it doesn't hurt. There are other tests
 that don't use .spec files as well.

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