BIND 10 #2977: Implement DnsClient Packet I/O.

BIND 10 Development do-not-reply at isc.org
Tue Jul 9 12:09:00 UTC 2013


#2977: Implement DnsClient Packet I/O.
-------------------------------------+-------------------------------------
            Reporter:  tmark         |                        Owner:
                Type:  enhancement   |  marcin
            Priority:  medium        |                       Status:
           Component:  dhcp-ddns     |  reviewing
            Keywords:                |                    Milestone:
           Sensitive:  0             |  Sprint-DHCP-20130717
         Sub-Project:  DHCP          |                   Resolution:
Estimated Difficulty:  0             |                 CVSS Scoring:
         Total Hours:  0             |              Defect Severity:  N/A
                                     |  Feature Depending on Ticket:  2975
                                     |          Add Hours to Ticket:  0
                                     |                    Internal?:  0
-------------------------------------+-------------------------------------
Changes (by tmark):

 * owner:  tmark => marcin


Comment:

 Review Comments:

 * I think launch test mode parameter should default to false.

 * dns_client.cc  - Comment states we aren't implementing TCP because it
 would require a bunch of new tests:
 {{{
 "
 // @todo At some point we may need to implement TCP. It should be straight
 // forward but would require a bunch of new unit tests. That's why we
 // currently disable TCP. Once implemented the check below should be
 "
 }}}

 Actually, we aren't doing it yet because we are doing a phased
 implementation.  I think it would be better to say "TCP support will be
 provided in a future release."

 * is it typical in PIMPL for the impl class members to be public?

 * should DNSClient::impl_ be a smart pointer?

 * you declare the copy and assignment operators private because DNSClient
 is a singleton.  It is not a singleton.   It has a public constructor
 which allows me to make any number of instances.  The real reason to hide
 them, is that the default implementations would make a copy of the
 DNSClient::impl_ which is a pointer.
 Either the reason we can't copy is wrong in the comment, or you need to
 make it a singleton.  I would prefer the former, I don't see a reason for
 it to be a singleton.


 * dns_client_unittests.cc  line 120 is too long

 * dns_client_unittests.cc "allowed. This why we will"  change "why" to
 "way"

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


More information about the bind10-tickets mailing list