BIND 10 #1584: auth::Query NSEC3 support: Wildcard answer case

BIND 10 Development do-not-reply at isc.org
Thu Feb 16 23:15:10 UTC 2012


#1584: auth::Query NSEC3 support: Wildcard answer case
-------------------------------------+-------------------------------------
                   Reporter:         |                 Owner:  haikuo
  jinmei                             |                Status:  accepted
                       Type:  task   |             Milestone:
                   Priority:  major  |  Sprint-20120221
                  Component:         |            Resolution:
  b10-auth                           |             Sensitive:  0
                   Keywords:         |           Sub-Project:  DNS
            Defect Severity:  N/A    |  Estimated Difficulty:  5
Feature Depending on Ticket:  NSEC3  |           Total Hours:  0
        Add Hours to Ticket:  0      |
                  Internal?:  0      |
-------------------------------------+-------------------------------------

Comment (by jinmei):

 As we briefly discussed in the daily call, I reviewed the latest
 snapshot of the trac1584 branch anyway.  I completed the review, and
 actually even addressed my own comments myself.  I didn't want to
 steal the branch owned by someone else, but in this particular case
 we rely on the completion of this branch for including the major
 feature in the next release, and apparently we are running out of our
 time.  So I hope it's acceptable for you.

 (Most of) my own updates are made in a separate branch (to minimize
 possible conflicts), trac1584review.  I've created the branch from a
 recent master, and merged the latest available version of trac1584 (at
 commit c9b1b85).

 Now, if this "takeover" is acceptable for you, could you review the
 changes I made in the trac1584review?  If you don't think you have
 enough time to complete the review within this week, please ask
 someone else for taking over it.

 Comments on the original trac1584 branch (and explanation on my own
 changes) follow:

 '''general'''

 - There are many violations of the coding guidelines: indentation, too
   long lines, (although not explicitly documented) redundant white
   space characters at end-of-lines or in blank lines, inconsistent
   space style like "if (foo){" (we add space between ')' and '{').
   If you have not done so, please read the guideline carefully.  For
   indentation, maybe you want to adjust your editor configuration (you
   seem to insert 8-space for a "tab")
 - conventionally we add "[ticket number]" at the beginning of commit
   logs.  Please follow that convention.  You can do that, e.g., by
   adding this to .git/hooks/prepare-commit-msg (this is what I'm
   doing):
 {{{
 #!/bin/sh

 BRANCH=`git branch | sed -ne 's/\* trac\(.*\)/\1/p'`
 if test -z $BRANCH; then
         BRANCH=master
 fi
 /bin/echo -n "[$BRANCH] " > "$1.msg"
 cat "$1" >> "$1.msg"
 cat "$1.msg" > "$1"
 }}}
 - As I pointed out in the first comment message, we adopt test driven
   development.  So, basically, it cannot happen you only commit main
   code without tests.  In the case of this branch, the first change
   would be something like commit 31ae358.  We then confirm it fails
   with the current implementation, then extend the main code.
   Apparently your work flow is opposite.  Please make sure write tests
   first next time.  If you are not familiar with TDD, please ask us or
   someone in CNNIC who have worked on BIND10 before.

 '''query.cc'''

 - the check for isWildcard() in addWildcardProof() is redundant (I
   fixed it)
 - on further look, I noticed we actually didn't have to construct the
   next closer name to the wildcard.  By definition it must be equal to
   the next closer of the closest encloser for the qname.  So I
   simplified the code by removing the unnecessary stuff.
 - otherwise the code generally looked correct, but as I did in #1583,
   I had Message::addRRset() handle the unexpected case where
   next_proof is NULL.  That simplifies the code for the normal case.

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


More information about the bind10-tickets mailing list