[trac501] Address review comments

src/lib/nsas/glue_hints.cc

@@ -63,44 +63,6 @@ namespace {
             rdi->next();
         }
     }
-
-    // Add the A and AAAA records from the given message for the given
-    // NS name to the relevant address vector
-    // (A rrsets are added to addresses_v4, AAAA rrsets are added to
-    // addresses_v6).
-    void
-    addGlueForName(std::vector<NameserverAddress>& addresses_v4,
-                   std::vector<NameserverAddress>& addresses_v6,
-                   const Name& name, const Message& message)
-    {
-        for (RRsetIterator rssi = message.beginSection(Message::SECTION_ADDITIONAL);
-             rssi != message.endSection(Message::SECTION_ADDITIONAL);
-             ++rssi) {
-            if ((*rssi)->getName() == name) {
-                if ((*rssi)->getType() == RRType::A()) {
-                    addRRset(addresses_v4, *rssi);
-                } else if ((*rssi)->getType() == RRType::AAAA()) {
-                    addRRset(addresses_v6, *rssi);
-                }
-            }
-        }
-    }
-
-    // Add the glue for the given NS RRset in the message to the
-    // relevant vectors.
-    void
-    addGlueForRRset(std::vector<NameserverAddress>& addresses_v4,
-                    std::vector<NameserverAddress>& addresses_v6,
-                    const RRsetPtr rrset, const Message& message)
-    {
-        RdataIteratorPtr rdi = rrset->getRdataIterator();
-        while (!rdi->isLast()) {
-            isc::dns::Name name(dynamic_cast<const rdata::generic::NS&>(
-                            rdi->getCurrent()).getNSName());
-            addGlueForName(addresses_v4, addresses_v6, name, message);
-            rdi->next();
-        }
-    }
 }
 
 namespace isc {
@@ -109,14 +71,12 @@ namespace nsas {
 GlueHints::GlueHints(const std::string& zone_name,
                      const isc::dns::Message& delegation_message)
 {
-    std::cout << delegation_message.toText();
     for (RRsetIterator rssi = delegation_message.beginSection(Message::SECTION_AUTHORITY);
          rssi != delegation_message.endSection(Message::SECTION_AUTHORITY);
          ++rssi) {
         if ((*rssi)->getType() == RRType::NS() &&
             (*rssi)->getName().toText() == zone_name) {
-            addGlueForRRset(addresses_v4, addresses_v6,
-                            *rssi, delegation_message);
+            addGlueForRRset(*rssi, delegation_message);
         }
     }
 }
@@ -165,5 +125,40 @@ GlueHints::getGlue(AddressFamily family) const {
     }
 }
 
+// Add the A and AAAA records from the given message for the given
+// NS name to the relevant address vector
+// (A rrsets are added to addresses_v4, AAAA rrsets are added to
+// addresses_v6).
+void
+GlueHints::addGlueForName(const Name& name, const Message& message)
+{
+    for (RRsetIterator rssi = message.beginSection(Message::SECTION_ADDITIONAL);
+         rssi != message.endSection(Message::SECTION_ADDITIONAL);
+         ++rssi) {
+        if ((*rssi)->getName() == name) {
+            if ((*rssi)->getType() == RRType::A()) {
+                addRRset(addresses_v4, *rssi);
+            } else if ((*rssi)->getType() == RRType::AAAA()) {
+                addRRset(addresses_v6, *rssi);
+            }
+        }
+    }
+}
+
+// Add the glue for the given NS RRset in the message to the
+// relevant vectors.
+void
+GlueHints::addGlueForRRset(const RRsetPtr rrset, const Message& message)
+{
+    RdataIteratorPtr rdi = rrset->getRdataIterator();
+    while (!rdi->isLast()) {
+        isc::dns::Name name(dynamic_cast<const rdata::generic::NS&>(
+                        rdi->getCurrent()).getNSName());
+        addGlueForName(name, message);
+        rdi->next();
+    }
+}
+
+
 } // namespace nsas
 } // namespace isc

src/lib/nsas/glue_hints.h

@@ -55,6 +55,10 @@ public:
     NameserverAddress getGlue(AddressFamily family) const;
 
 private:
+    void addGlueForName(const isc::dns::Name& name,
+                        const isc::dns::Message& message);
+    void addGlueForRRset(const isc::dns::RRsetPtr rrset,
+                         const isc::dns::Message& message);
 
     std::vector<NameserverAddress> addresses_v4;
     std::vector<NameserverAddress> addresses_v6;

src/lib/nsas/nameserver_address.h

@@ -65,7 +65,7 @@ public:
     /// request is processing.
     /// \param index The address's index in NameserverEntry's addresses vector
     /// \param family Address family, V4_ONLY or V6_ONLY
-    NameserverAddress(const boost::shared_ptr<NameserverEntry> nameserver,
+    NameserverAddress(const boost::shared_ptr<NameserverEntry>& nameserver,
         const AddressEntry& address, AddressFamily family):
         ns_(nameserver), address_(address), family_(family)
     {

src/lib/resolve/recursive_query.cc

@@ -294,7 +294,7 @@ private:
             isc::resolve::ResponseClassifier::classify(
                 question_, incoming, cname_target, cname_count_, true);
 
-        bool found_ns_address = false;
+        bool found_ns = false;
             
         switch (category) {
         case isc::resolve::ResponseClassifier::ANSWER:
@@ -348,18 +348,18 @@ private:
             // classifier should have error'd)
             // TODO: should we check if it really is subzone?
             for (RRsetIterator rrsi = incoming.beginSection(Message::SECTION_AUTHORITY);
-                 rrsi != incoming.endSection(Message::SECTION_AUTHORITY) && !found_ns_address;
+                 rrsi != incoming.endSection(Message::SECTION_AUTHORITY) && !found_ns;
                  ++rrsi) {
                 ConstRRsetPtr rrs = *rrsi;
                 if (rrs->getType() == RRType::NS()) {
                     cur_zone_ = rrs->getName().toText();
                     dlog("Referred to zone " + cur_zone_);
-                    found_ns_address = true;
+                    found_ns = true;
                     break;
                 }
             }
 
-            if (found_ns_address) {
+            if (found_ns) {
                 // next resolver round
                 // we do NOT use doLookup() here, but send() (i.e. we
                 // skip the cache), since if we had the final answer