Browse Source

[2977] Address comments from the second round of review.

Marcin Siodelski 12 years ago
parent
commit
a8a6595db6
3 changed files with 10 additions and 8 deletions
  1. 2 4
      src/bin/d2/dns_client.cc
  2. 5 2
      src/bin/d2/dns_client.h
  3. 3 2
      src/bin/d2/tests/dns_client_unittests.cc

+ 2 - 4
src/bin/d2/dns_client.cc

@@ -81,10 +81,8 @@ DNSClientImpl::DNSClientImpl(D2UpdateMessagePtr& response_placeholder,
     : in_buf_(new OutputBuffer(DEFAULT_BUFFER_SIZE)),
       response_(response_placeholder), callback_(callback), proto_(proto) {
 
-    // @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
-    // removed.
+    // @todo Currently we only support UDP. The support for TCP is planned for
+    // the future release.
     if (proto_ == DNSClient::TCP) {
         isc_throw(isc::NotImplemented, "TCP is currently not supported as a"
                   << " Transport protocol for DNS Updates; please use UDP");

+ 5 - 2
src/bin/d2/dns_client.h

@@ -109,8 +109,11 @@ public:
     ///
     /// @name Copy constructor and assignment operator
     ///
-    /// Copy constructor and assignment operator are private because
-    /// @c DNSClient is a singleton class and its instance should not be copied.
+    /// Copy constructor and assignment operator are private because there are
+    /// no use cases when @DNSClient instance will need to be copied. Also, it
+    /// is desired to avoid copying @DNSClient::impl_ pointer and external
+    /// callbacks.
+    ///
     //@{
 private:
     DNSClient(const DNSClient& source);

+ 3 - 2
src/bin/d2/tests/dns_client_unittests.cc

@@ -117,7 +117,8 @@ public:
 
                 ASSERT_TRUE(response_);
                 EXPECT_EQ(D2UpdateMessage::RESPONSE, response_->getQRFlag());
-                ASSERT_EQ(1, response_->getRRCount(D2UpdateMessage::SECTION_ZONE));
+                ASSERT_EQ(1,
+                          response_->getRRCount(D2UpdateMessage::SECTION_ZONE));
                 D2ZonePtr zone = response_->getZone();
                 ASSERT_TRUE(zone);
                 EXPECT_EQ("example.com.", zone->getName().toText());
@@ -211,7 +212,7 @@ public:
         ASSERT_NO_THROW(message.setRcode(Rcode(Rcode::NOERROR_CODE)));
         ASSERT_NO_THROW(message.setZone(Name("example.com"), RRClass::IN()));
 
-        // Start with a valid timeout equal to maximal allowed. This why we will
+        // Start with a valid timeout equal to maximal allowed. This way we will
         // ensure that doUpdate doesn't throw an exception for valid timeouts.
         unsigned int timeout = DNSClient::getMaxTimeout();
         EXPECT_NO_THROW(dns_client_->doUpdate(service_, IOAddress(TEST_ADDRESS),