BIND 10 #327: early work on recursion

[BIND 10 Development]

#327: early work on recursion
---------------------------+------------------------------------------------
      Reporter:  each      |        Owner:  each                 
          Type:  defect    |       Status:  reviewing            
      Priority:  major     |    Milestone:  y2 12 month milestone
     Component:  recurser  |   Resolution:                       
      Keywords:            |    Sensitive:  0                    
Estimatedhours:  0.0       |        Hours:  0                    
      Billable:  1         |   Totalhours:  0                    
      Internal:  0         |  
---------------------------+------------------------------------------------

Comment(by each):

 Replying to [comment:9 stephen]:
 > '''src/lib/asiolink/ioaddress.h'''
 > OK, but shouldn't the toText() method be declared virtual (given that
 getFamily() is)?

 OK.

 >
 > '''src/bin/auth/auth_srv.h''' and '''src/bin/recurse/recursor.h'''
 > Methods have been documented, but processMessage() header needs
 arguments documenting.
 >
 > Even though setVerbose() and getVerbose() methods are trivial, shouldn't
 we document them individually (and their parameters) for doxygen?
 >
 > '''src/bin/auth/auth_srv.cc'''
 > Messages are being written to cerr; a proper logging system is required.

 Agreed, but out of scope for this ticket.

 > '''src/bin/auth/tests/mockups.h'''
 > Class !MockSession: it would be useful to identify the methods (by means
 of comments) that have been added over and above the !AbstractSession.

 I'm not actually sure, this part isn't my code, but I'll attempt it.

 > Class !MockSession: Why are the "sent_msg" and "msg_destination"
 elements public?

 Because they're used outside MockSession, see for example
 auth_srv_unittest.cc line 391.  But I can add "getter" functions.

 > '''src/bin/auth/main.cc''' and '''src/bin/recurse/main.cc'''
 > The example command line in the usage message should include "[-u]" as
 an option (although it is listed as an option in the text below).

 Done.

 > Default port is still 5300 (and not 53).  If this is only because it is
 an early version for testing, a comment should be added to that effect.

 Okay.  Added the comment above the definition of DNSPORT and the usage
 message now prints DNSPORT rather than having 5300 hardcoded.

 > '''src/bin/recurse/recursor.cc'''
 > Member variables are still public.

 RecursorImpl members can be seen by Recursor, yes.  That doesn't seem
 wrong for a pimpl architecture, but we can add getter/setter functions if
 you like.  Or define Recursor as a friend class (but we've been avoiding
 friend, as I understand it).

 For the time being I just made all the members private that aren't
 currently being used outside the RecursorImpl class.  Ditto for
 AuthSrvImpl.

 Committed changes to r3310.

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