BIND 10 #2124: RFC 6594 for SSHFP

BIND 10 Development do-not-reply at isc.org
Fri Jul 27 07:44:28 UTC 2012


#2124: RFC 6594 for SSHFP
-------------------------------------+-------------------------------------
                   Reporter:         |                 Owner:  jinmei
  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 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.

 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).

 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.

 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.

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


More information about the bind10-tickets mailing list