[trac497] final review comments

as to copySection, it's now called appendSection, and works the other
way around (i.e. it copies from a source message into the object,
instead of copying from the object to a target message), which feels
more natural

src/lib/asiolink/asiolink.cc

@@ -440,8 +440,8 @@ private:
                 return true;
             }
 
-            incoming.copySection(*answer_message_,
-                Message::SECTION_ANSWER);
+            answer_message_->appendSection(Message::SECTION_ANSWER,
+                                           incoming);
             setZoneServersToRoot();
 
             question_ = Question(cname_target, question_.getClass(),
@@ -574,7 +574,7 @@ public:
                 boost::lexical_cast<string>(upstream_root_->size()) + 
                 "\n");
             for(AddressVector::iterator it = upstream_root_->begin();
-                it < upstream_root_->end(); it++) {
+                it < upstream_root_->end(); ++it) {
             zone_servers_.push_back(addr_t(it->first,it->second));
             dlog("Put " + zone_servers_.back().first + "into root list\n");
             }

src/lib/dns/message.cc

@@ -777,21 +777,22 @@ Message::clear(Mode mode) {
 }
 
 void
-Message::copySection(Message& target, const Section section) const {
+Message::appendSection(const Section section, const Message& source) {
     if (section >= MessageImpl::NUM_SECTIONS) {
         isc_throw(OutOfRange, "Invalid message section: " << section);
     }
 
     if (section == SECTION_QUESTION) {
-        BOOST_FOREACH(QuestionPtr q, impl_->questions_) {
-            std::cout << "[XX] copy question " << q << std::endl;
-            target.addQuestion(q);
+        for (QuestionIterator qi = source.beginQuestion();
+             qi != source.endQuestion();
+             ++qi) {
+            addQuestion(*qi);
         }
     } else {
-        std::cout << "[XX] copy section " << section << std::endl;
-        BOOST_FOREACH(RRsetPtr r, impl_->rrsets_[section]) {
-            std::cout << "[XX] copy rrset " << r << std::endl;
-            target.addRRset(section, r, false);
+        for (RRsetIterator rrsi = source.beginSection(section);
+             rrsi != source.endSection(section);
+             ++rrsi) {
+            addRRset(section, *rrsi);
         }
     }
 }

src/lib/dns/message.h

@@ -498,12 +498,12 @@ public:
     /// specified mode.
     void clear(Mode mode);
 
-    /// \brief Adds all rrsets in the given section to the same section
-    /// in target
+    /// \brief Adds all rrsets from the source the given section in the
+    /// source message to the same section of this message
     ///
-    /// \param target The target Message
-    /// \param section the section to copy
-    void copySection(Message& target, const Section section) const;
+    /// \param section the section to append
+    /// \param target The source Message
+    void appendSection(const Section section, const Message& source);
 
     /// \brief Prepare for making a response from a request.
     ///

src/lib/dns/tests/message_unittest.cc

@@ -380,21 +380,21 @@ TEST_F(MessageTest, badEndSection) {
     EXPECT_THROW(message_render.endSection(bogus_section), OutOfRange);
 }
 
-TEST_F(MessageTest, copySection) {
+TEST_F(MessageTest, appendSection) {
     Message target(Message::RENDER);
 
     // Section check
-    EXPECT_THROW(message_render.copySection(target, bogus_section),
+    EXPECT_THROW(target.appendSection(bogus_section, message_render),
                  OutOfRange);
 
     // Make sure nothing is copied if there is nothing to copy
-    message_render.copySection(target, Message::SECTION_QUESTION);
+    target.appendSection(Message::SECTION_QUESTION, message_render);
     EXPECT_EQ(0, target.getRRCount(Message::SECTION_QUESTION));
-    message_render.copySection(target, Message::SECTION_ANSWER);
+    target.appendSection(Message::SECTION_ANSWER, message_render);
     EXPECT_EQ(0, target.getRRCount(Message::SECTION_ANSWER));
-    message_render.copySection(target, Message::SECTION_AUTHORITY);
+    target.appendSection(Message::SECTION_AUTHORITY, message_render);
     EXPECT_EQ(0, target.getRRCount(Message::SECTION_AUTHORITY));
-    message_render.copySection(target, Message::SECTION_ADDITIONAL);
+    target.appendSection(Message::SECTION_ADDITIONAL, message_render);
     EXPECT_EQ(0, target.getRRCount(Message::SECTION_ADDITIONAL));
 
     // Now add some data, copy again, and see if it got added
@@ -405,20 +405,20 @@ TEST_F(MessageTest, copySection) {
     message_render.addRRset(Message::SECTION_ADDITIONAL, rrset_a);
     message_render.addRRset(Message::SECTION_ADDITIONAL, rrset_aaaa);
 
-    message_render.copySection(target, Message::SECTION_QUESTION);
+    target.appendSection(Message::SECTION_QUESTION, message_render);
     EXPECT_EQ(1, target.getRRCount(Message::SECTION_QUESTION));
 
-    message_render.copySection(target, Message::SECTION_ANSWER);
+    target.appendSection(Message::SECTION_ANSWER, message_render);
     EXPECT_EQ(2, target.getRRCount(Message::SECTION_ANSWER));
     EXPECT_TRUE(target.hasRRset(Message::SECTION_ANSWER, test_name,
         RRClass::IN(), RRType::A()));
 
-    message_render.copySection(target, Message::SECTION_AUTHORITY);
+    target.appendSection(Message::SECTION_AUTHORITY, message_render);
     EXPECT_EQ(2, target.getRRCount(Message::SECTION_AUTHORITY));
     EXPECT_TRUE(target.hasRRset(Message::SECTION_AUTHORITY, test_name,
         RRClass::IN(), RRType::A()));
 
-    message_render.copySection(target, Message::SECTION_ADDITIONAL);
+    target.appendSection(Message::SECTION_ADDITIONAL, message_render);
     EXPECT_EQ(3, target.getRRCount(Message::SECTION_ADDITIONAL));
     EXPECT_TRUE(target.hasRRset(Message::SECTION_ADDITIONAL, test_name,
         RRClass::IN(), RRType::A()));
@@ -428,7 +428,7 @@ TEST_F(MessageTest, copySection) {
     // One more test, test to see if the section gets added, not replaced
     Message source2(Message::RENDER);
     source2.addRRset(Message::SECTION_ANSWER, rrset_aaaa);
-    source2.copySection(target, Message::SECTION_ANSWER);
+    target.appendSection(Message::SECTION_ANSWER, source2);
     EXPECT_EQ(3, target.getRRCount(Message::SECTION_ANSWER));
     EXPECT_TRUE(target.hasRRset(Message::SECTION_ANSWER, test_name,
         RRClass::IN(), RRType::A()));

src/lib/resolve/resolve.cc

@@ -47,9 +47,9 @@ makeErrorMessage(MessagePtr answer_message,
 void copyResponseMessage(const Message& source, MessagePtr target) {
     target->setRcode(source.getRcode());
 
-    source.copySection(*target, Message::SECTION_ANSWER);
-    source.copySection(*target, Message::SECTION_AUTHORITY);
-    source.copySection(*target, Message::SECTION_ADDITIONAL);
+    target->appendSection(Message::SECTION_ANSWER, source);
+    target->appendSection(Message::SECTION_AUTHORITY, source);
+    target->appendSection(Message::SECTION_ADDITIONAL, source);
 }