BIND 10 #497: CNAME processing / CNAME loop detection (reception)

[BIND 10 Development]

#497: CNAME processing / CNAME loop detection (reception)
-------------------------------------+-------------------------------------
                 Reporter:  shane    |                Owner:  jelte
                     Type:           |               Status:  reviewing
  enhancement                        |            Milestone:  R-Team-
                 Priority:  major    |  Sprint-20110208
                Component:           |           Resolution:
  resolver                           |            Sensitive:  0
                 Keywords:           |  Add Hours to Ticket:  0
Estimated Number of Hours:  3.0      |          Total Hours:  0
                Billable?:  1        |
                Internal?:  0        |
-------------------------------------+-------------------------------------
Changes (by stephen):

 * owner:  stephen => jelte


Comment:

 '''src/lib/asiolink/asiolink.cc'''[[BR]]
 Should perhaps add a comment to the declaration of cname_count_ in
 !RunningQuery.

 handleRecursiveAnswer(): in the "switch" statement, would not a "default"
 be better than explicitly listing the remaining codes responseClassifier
 could return?  (There is always a chance that !ResponseClassifier is
 altered to extend the number of codes returned - in which case, we
 probably don't want to abort with a fatal error (as the comment suggests)
 if we exit the switch.)

 Comment: I suggest that comments/suggestions in the comments be
 highlighted with a "TODO:" or a
 "XXX:" - programming environments highlight these, and they are easy to
 search for.  Else we run the risk of such comments being forgotten.

 setZoneServersToRoot(): I'm not sure how BOOST_FOREACH could improve over
 the simple loop, other than to pre-increment the iterator instead of post-
 incrementing it.  (Means the iterator code does not have to save a copy of
 the iterator before it was incremented - although it is quite possible
 that the optimiser has optimised that code away.)


 '''src/lib/asiolink/tests/asiolink_unittest.cc'''[[BR]]
 forwardClientTimeout(): when creating a !RecursiveQuery, the number of
 retries has been increased to 4, but the comment above it still refers to
 three retries.



 '''src/lib/resolve/resolve.h'''[[BR]]
 Comment: copySection really seems to belong to the isc::dns::Message
 class.  In fact, it does occur to me that if we were to make the section
 numbers bit masks, then we could rename the method copySections() and do
 all the copying in one call by passing in the logical OR of the sections
 we wish to copy.

 Comment: would "copyResponseMessage()" be a better name than
 "copyAnswerMessage()", as the fact that there is an "answer" section may
 make the name slightly confusing.


 '''src/lib/resolve/response_classifier.h'''[[BR]]
 OK, although the "TODO" about code being not being used in this form can
 be removed as the predicted changes have now been made.

 Comment: descriptions of new parameters not in same style as the other
 parameter descriptions.

 '''!ChangeLog Entries'''[[BR]]
 These are fine.

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