BIND 10 trac658, updated. 539dcb20ce9dc94c46ce424bd2bfece5c44a7daa [trac658] address review comments

BIND 10 source code commits bind10-changes at lists.isc.org
Fri Mar 11 11:46:13 UTC 2011


The branch, trac658 has been updated
       via  539dcb20ce9dc94c46ce424bd2bfece5c44a7daa (commit)
      from  f6556f773732939bf68129e8c1fa10e84f8f2e36 (commit)

Those revisions listed above that are new to this repository have
not appeared on any other notification email; so we list those
revisions in full, below.

- Log -----------------------------------------------------------------
commit 539dcb20ce9dc94c46ce424bd2bfece5c44a7daa
Author: Jelte Jansen <jelte at isc.org>
Date:   Fri Mar 11 12:45:52 2011 +0100

    [trac658] address review comments

-----------------------------------------------------------------------

Summary of changes:
 src/lib/asiolink/io_fetch.cc         |   33 +++++++++++++++++++++------------
 src/lib/asiolink/io_fetch.h          |   28 +++++++++++++++-------------
 src/lib/dns/tests/buffer_unittest.cc |    4 ++++
 3 files changed, 40 insertions(+), 25 deletions(-)

-----------------------------------------------------------------------
diff --git a/src/lib/asiolink/io_fetch.cc b/src/lib/asiolink/io_fetch.cc
index 04993f4..33f81cb 100644
--- a/src/lib/asiolink/io_fetch.cc
+++ b/src/lib/asiolink/io_fetch.cc
@@ -56,6 +56,20 @@ namespace {
 
         return (data);
     }
+
+    // Checks if the response we received was ok;
+    // - data contains the buffer we read, as well as the address
+    // we sent to and the address we received from.
+    // length is provided by the operator() in IOFetch.
+    // Addresses must match, number of octets read must be at least
+    // 2, and the first two octets must match the qid of the message
+    // we sent.
+    bool responseOK(const asiolink::IOFetch::IOFetchData& data,
+                    size_t length) {
+        return (data.remote_snd == data.remote_rcv &&
+                length >= 2 &&
+                readUint16(data.data.get(), length) == data.qid);
+    }
 }
 
 namespace asiolink {
@@ -99,7 +113,7 @@ IOFetch::operator()(error_code ec, size_t length) {
             data_->cumulative = 0;
 
             dlog("Sending " + msg.toText() + " to " +
-                data_->remote->getAddress().toText());
+                data_->remote_snd->getAddress().toText());
         }
 
 
@@ -114,15 +128,15 @@ IOFetch::operator()(error_code ec, size_t length) {
 
         // Open a connection to the target system.  For speed, if the operation
         // was completed synchronously (i.e. UDP operation) we bypass the yield.
-        if (data_->socket->open(data_->remote.get(), *this)) {
+        if (data_->socket->open(data_->remote_snd.get(), *this)) {
             CORO_YIELD;
         }
 
-        while (true) {
+        do {
             // Begin an asynchronous send, and then yield.  When the send completes
             // send completes, we will resume immediately after this point.
             CORO_YIELD data_->socket->asyncSend(data_->msgbuf->getData(),
-                data_->msgbuf->getLength(), data_->remote.get(), *this);
+                data_->msgbuf->getLength(), data_->remote_snd.get(), *this);
 
             // Now receive the response.  Since TCP may not receive the entire
             // message in one operation, we need to loop until we have received
@@ -134,24 +148,19 @@ IOFetch::operator()(error_code ec, size_t length) {
             do {
                 CORO_YIELD data_->socket->asyncReceive(data_->data.get(),
                     static_cast<size_t>(MAX_LENGTH), data_->cumulative,
-                    data_->remote.get(), *this);
+                    data_->remote_rcv.get(), *this);
             } while (!data_->socket->receiveComplete(data_->data.get(), length,
                 data_->cumulative));
 
             // The message is not rendered yet, so we can't print it easily
-            dlog("Received response from " + data_->remote->getAddress().toText());
+            dlog("Received response from " + data_->remote_rcv->getAddress().toText());
 
             // TODO: move the qid check to wherever we put demuxer (and all
             // DNS related knowledge out of this (as well as the dupe uint16
             // read code)
             // This is just a quick way to check it, if we get the right ID
             // (TODO: from the right address), break the receive-loop
-            if (readUint16(data_->data.get(), length) == data_->qid) {
-                break;
-            }  else {
-                dlog("Wrong QID in response, waiting for next one");
-            }
-        }
+        } while (!responseOK(*data_, length));
 
         /// Copy the answer into the response buffer.  (TODO: If the
         /// OutputBuffer object were made to meet the requirements of
diff --git a/src/lib/asiolink/io_fetch.h b/src/lib/asiolink/io_fetch.h
index a64fc22..484e62d 100644
--- a/src/lib/asiolink/io_fetch.h
+++ b/src/lib/asiolink/io_fetch.h
@@ -117,18 +117,19 @@ public:
         // a shared pointer here is merely to ensure deletion when the data
         // object is deleted.
         boost::shared_ptr<IOAsioSocket<IOFetch> > socket;
-                                                ///< Socket to use for I/O
-        boost::shared_ptr<IOEndpoint> remote;   ///< Where the fetch was sent
-        isc::dns::Question          question;   ///< Question to be asked
-        isc::dns::OutputBufferPtr   msgbuf;     ///< Wire buffer for question
-        isc::dns::OutputBufferPtr   buffer;     ///< Received data held here
-        boost::shared_array<char>   data;       ///< Temporary array for data
-        IOFetch::Callback*          callback;   ///< Called on I/O Completion
-        size_t                      cumulative; ///< Cumulative received amount
-        bool                        stopped;    ///< Have we stopped running?
-        asio::deadline_timer        timer;      ///< Timer to measure timeouts
-        int                         timeout;    ///< Timeout in ms
-        isc::dns::qid_t             qid;        ///< The QID set in the query
+                                                 ///< Socket to use for I/O
+        boost::shared_ptr<IOEndpoint> remote_snd;///< Where the fetch is sent
+        boost::shared_ptr<IOEndpoint> remote_rcv;///< Where the response came from
+        isc::dns::Question          question;    ///< Question to be asked
+        isc::dns::OutputBufferPtr   msgbuf;      ///< Wire buffer for question
+        isc::dns::OutputBufferPtr   buffer;      ///< Received data held here
+        boost::shared_array<char>   data;        ///< Temporary array for data
+        IOFetch::Callback*          callback;    ///< Called on I/O Completion
+        size_t                      cumulative;  ///< Cumulative received amount
+        bool                        stopped;     ///< Have we stopped running?
+        asio::deadline_timer        timer;       ///< Timer to measure timeouts
+        int                         timeout;     ///< Timeout in ms
+        isc::dns::qid_t             qid;         ///< The QID set in the query
 
         /// \brief Constructor
         ///
@@ -159,10 +160,11 @@ public:
                 static_cast<IOAsioSocket<IOFetch>*>(
                     new TCPSocket<IOFetch>(service))
                 ),
-            remote((protocol == IPPROTO_UDP) ?
+            remote_snd((protocol == IPPROTO_UDP) ?
                 static_cast<IOEndpoint*>(new UDPEndpoint(address, port)) :
                 static_cast<IOEndpoint*>(new TCPEndpoint(address, port))
                 ),
+            remote_rcv(remote_snd),
             question(query),
             msgbuf(new isc::dns::OutputBuffer(512)),
             buffer(buff),
diff --git a/src/lib/dns/tests/buffer_unittest.cc b/src/lib/dns/tests/buffer_unittest.cc
index fb4c86c..bba5757 100644
--- a/src/lib/dns/tests/buffer_unittest.cc
+++ b/src/lib/dns/tests/buffer_unittest.cc
@@ -144,6 +144,10 @@ TEST_F(BufferTest, outputBufferWriteat) {
     EXPECT_EQ(2, *(cp + 2));
     EXPECT_EQ(3, *(cp + 3));
 
+    EXPECT_THROW(obuffer.writeUint8At(data16, 5),
+                 isc::dns::InvalidBufferPosition);
+    EXPECT_THROW(obuffer.writeUint8At(data16, 4),
+                 isc::dns::InvalidBufferPosition);
     EXPECT_THROW(obuffer.writeUint16At(data16, 3),
                  isc::dns::InvalidBufferPosition);
     EXPECT_THROW(obuffer.writeUint16At(data16, 4),




More information about the bind10-changes mailing list