[trac404] Remove duplicate code

src/lib/dns/messagerenderer.cc

@@ -16,7 +16,6 @@
 #include <cassert>
 #include <set>
 
-#include <dns/buffer.h>
 #include <dns/name.h>
 #include <dns/messagerenderer.h>
 
@@ -150,12 +149,10 @@ struct MessageRenderer::MessageRendererImpl {
     ///
     /// \param buffer An \c OutputBuffer object to which wire format data is
     /// written.
-    MessageRendererImpl(OutputBuffer& buffer) :
-        buffer_(buffer), nbuffer_(Name::MAX_WIRE), msglength_limit_(512),
+    MessageRendererImpl() :
+        nbuffer_(Name::MAX_WIRE), msglength_limit_(512),
         truncated_(false), compress_mode_(MessageRenderer::CASE_INSENSITIVE)
     {}
-    /// The buffer that holds the entire DNS message.
-    OutputBuffer& buffer_;
     /// A local working buffer to convert each given name into wire format.
     /// This could be a local variable of the \c writeName() method, but
     /// we keep it in the class so that we can reuse it and avoid construction
@@ -174,7 +171,8 @@ struct MessageRenderer::MessageRendererImpl {
 };
 
 MessageRenderer::MessageRenderer(OutputBuffer& buffer) :
-    impl_(new MessageRendererImpl(buffer))
+    AbstractMessageRenderer(buffer),
+    impl_(new MessageRendererImpl)
 {}
 
 MessageRenderer::~MessageRenderer() {
@@ -183,17 +181,17 @@ MessageRenderer::~MessageRenderer() {
 
 void
 MessageRenderer::skip(const size_t len) {
-    impl_->buffer_.skip(len);
+    buffer_.skip(len);
 }
 
 void
 MessageRenderer::trim(const size_t len) {
-    impl_->buffer_.trim(len);
+    buffer_.trim(len);
 }
 
 void
 MessageRenderer::clear() {
-    impl_->buffer_.clear();
+    buffer_.clear();
     impl_->nbuffer_.clear();
     impl_->nodeset_.clear();
     impl_->msglength_limit_ = 512;
@@ -201,41 +199,6 @@ MessageRenderer::clear() {
     impl_->compress_mode_ = CASE_INSENSITIVE;
 }
 
-void
-MessageRenderer::writeUint8(const uint8_t data) {
-    impl_->buffer_.writeUint8(data);
-}
-
-void
-MessageRenderer::writeUint16(const uint16_t data) {
-    impl_->buffer_.writeUint16(data);
-}
-
-void
-MessageRenderer::writeUint16At(const uint16_t data, const size_t pos) {
-    impl_->buffer_.writeUint16At(data, pos);
-}
-
-void
-MessageRenderer::writeUint32(const uint32_t data) {
-    impl_->buffer_.writeUint32(data);
-}
-
-void
-MessageRenderer::writeData(const void* const data, const size_t len) {
-    impl_->buffer_.writeData(data, len);
-}
-
-const void*
-MessageRenderer::getData() const {
-    return (impl_->buffer_.getData());
-}
-
-size_t
-MessageRenderer::getLength() const {
-    return (impl_->buffer_.getLength());
-}
-
 size_t
 MessageRenderer::getLengthLimit() const {
     return (impl_->msglength_limit_);
@@ -291,15 +254,15 @@ MessageRenderer::writeName(const Name& name, const bool compress) {
     }
 
     // Record the current offset before extending the buffer.
-    const size_t offset = impl_->buffer_.getLength();
+    const size_t offset = buffer_.getLength();
     // Write uncompress part...
-    impl_->buffer_.writeData(impl_->nbuffer_.getData(),
+    buffer_.writeData(impl_->nbuffer_.getData(),
                              compress ? i : impl_->nbuffer_.getLength());
     if (compress && n != notfound) {
         // ...and compression pointer if available.
         uint16_t pointer = (*n).pos_;
         pointer |= Name::COMPRESS_POINTER_MARK16;
-        impl_->buffer_.writeUint16(pointer);
+        buffer_.writeUint16(pointer);
     }
 
     // Finally, add to the set the newly rendered name and its ancestors that
@@ -311,7 +274,7 @@ MessageRenderer::writeName(const Name& name, const bool compress) {
         if (offset + j > Name::MAX_COMPRESS_POINTER) {
             break;
         }
-        impl_->nodeset_.insert(NameCompressNode(*this, impl_->buffer_,
+        impl_->nodeset_.insert(NameCompressNode(*this, buffer_,
                                                 offset + j,
                                                 impl_->nbuffer_.getLength() -
                                                 j));

src/lib/dns/messagerenderer.h

@@ -15,10 +15,11 @@
 #ifndef __MESSAGERENDERER_H
 #define __MESSAGERENDERER_H 1
 
+#include <dns/buffer.h>
+
 namespace isc {
 namespace dns {
 // forward declarations
-class OutputBuffer;
 class Name;
 
 /// \brief The \c AbstractMessageRenderer class is an abstract base class
@@ -89,7 +90,15 @@ protected:
     ///
     /// This is intentionally defined as \c protected as this base class should
     /// never be instantiated (except as part of a derived class).
-    AbstractMessageRenderer() {}
+    AbstractMessageRenderer(OutputBuffer& buffer) :
+        buffer_(buffer)
+    {}
+    /// \short Buffer to store data
+    ///
+    /// It was decided that there's no need to have this in every subclass,
+    /// at last not now, and this reduces code size and gives compiler a better
+    /// chance to optimise.
+    OutputBuffer& buffer_;
 public:
     /// \brief The destructor.
     virtual ~AbstractMessageRenderer() {}
@@ -104,10 +113,14 @@ public:
     ///
     /// This method works exactly same as the same method of the \c OutputBuffer
     /// class; all notes for \c OutputBuffer apply.
-    virtual const void* getData() const = 0;
+    const void* getData() const {
+        return (buffer_.getData());
+    }
 
     /// \brief Return the length of data written in the internal buffer.
-    virtual size_t getLength() const = 0;
+    size_t getLength() const {
+        return (buffer_.getLength());
+    }
 
     /// \brief Return whether truncation has occurred while rendering.
     ///
@@ -196,13 +209,17 @@ public:
     /// \brief Write an unsigned 8-bit integer into the internal buffer.
     ///
     /// \param data The 8-bit integer to be written into the internal buffer.
-    virtual void writeUint8(uint8_t data) = 0;
+    void writeUint8(const uint8_t data) {
+        buffer_.writeUint8(data);
+    }
 
     /// \brief Write an unsigned 16-bit integer in host byte order into the
     /// internal buffer in network byte order.
     ///
     /// \param data The 16-bit integer to be written into the buffer.
-    virtual void writeUint16(uint16_t data) = 0;
+    void writeUint16(uint16_t data) {
+        buffer_.writeUint16(data);
+    }
 
     /// \brief Write an unsigned 16-bit integer in host byte order at the
     /// specified position of the internal buffer in network byte order.
@@ -215,13 +232,17 @@ public:
     ///
     /// \param data The 16-bit integer to be written into the internal buffer.
     /// \param pos The beginning position in the buffer to write the data.
-    virtual void writeUint16At(uint16_t data, size_t pos) = 0;
+    void writeUint16At(uint16_t data, size_t pos) {
+        buffer_.writeUint16At(data, pos);
+    }
 
     /// \brief Write an unsigned 32-bit integer in host byte order into the
     /// internal buffer in network byte order.
     ///
     /// \param data The 32-bit integer to be written into the buffer.
-    virtual void writeUint32(uint32_t data) = 0;
+    void writeUint32(uint32_t data) {
+        buffer_.writeUint32(data);
+    }
 
     /// \brief Copy an arbitrary length of data into the internal buffer
     /// of the renderer object.
@@ -230,7 +251,9 @@ public:
     ///
     /// \param data A pointer to the data to be copied into the internal buffer.
     /// \param len The length of the data in bytes.
-    virtual void writeData(const void *data, size_t len) = 0;
+    void writeData(const void *data, size_t len) {
+        buffer_.writeData(data, len);
+    }
 
     /// \brief Write a \c Name object into the internal buffer in wire format,
     /// with or without name compression.
@@ -273,8 +296,6 @@ public:
     MessageRenderer(OutputBuffer& buffer);
 
     virtual ~MessageRenderer();
-    virtual const void* getData() const;
-    virtual size_t getLength() const;
     virtual bool isTruncated() const;
     virtual size_t getLengthLimit() const;
     virtual CompressMode getCompressMode() const;
@@ -284,11 +305,6 @@ public:
     virtual void skip(size_t len);
     virtual void trim(size_t len);
     virtual void clear();
-    virtual void writeUint8(uint8_t data);
-    virtual void writeUint16(uint16_t data);
-    virtual void writeUint16At(uint16_t data, size_t pos);
-    virtual void writeUint32(uint32_t data);
-    virtual void writeData(const void *data, size_t len);
     virtual void writeName(const Name& name, bool compress = true);
 private:
     struct MessageRendererImpl;

src/lib/dns/rdatafields.cc

@@ -70,8 +70,9 @@ namespace {
 class RdataFieldComposer : public AbstractMessageRenderer {
 public:
     RdataFieldComposer(OutputBuffer& buffer) :
-        buffer_(buffer), truncated_(false), length_limit_(65535),
-        mode_(CASE_INSENSITIVE)
+        AbstractMessageRenderer(buffer),
+        truncated_(false), length_limit_(65535),
+        mode_(CASE_INSENSITIVE), last_data_pos_(0)
     {}
     virtual ~RdataFieldComposer() {}
     virtual const void* getData() const { return (buffer_.getData()); }
@@ -82,41 +83,15 @@ public:
     virtual void setTruncated() { truncated_ = true; }
     virtual void setLengthLimit(size_t len) { length_limit_ = len; }
     virtual void setCompressMode(CompressMode mode) { mode_ = mode; }
-    virtual void writeUint8(uint8_t data) {
-        buffer_.writeUint8(data);
-        if (fields_.empty() || fields_.back().type != RdataFields::DATA) {
-            fields_.push_back(RdataFields::FieldSpec(RdataFields::DATA, 0));
-        }
-        fields_.back().len += sizeof(data);
-    }
-    virtual void writeUint16(uint16_t data) {
-        buffer_.writeUint16(data);
-        if (fields_.empty() || fields_.back().type != RdataFields::DATA) {
-            fields_.push_back(RdataFields::FieldSpec(RdataFields::DATA, 0));
-        }
-        fields_.back().len += sizeof(data);
-    }
-    virtual void writeUint32(uint32_t data) {
-        buffer_.writeUint32(data);
-        if (fields_.empty() || fields_.back().type != RdataFields::DATA) {
-            fields_.push_back(RdataFields::FieldSpec(RdataFields::DATA, 0));
-        }
-        fields_.back().len += sizeof(data);
-    }
-    virtual void writeData(const void *data, size_t len) {
-        buffer_.writeData(data, len);
-        if (fields_.empty() || fields_.back().type != RdataFields::DATA) {
-            fields_.push_back(RdataFields::FieldSpec(RdataFields::DATA, 0));
-        }
-        fields_.back().len += len;
-    }
     virtual void writeName(const Name& name, bool compress) {
+        extendData();
         const RdataFields::Type field_type =
             compress ? RdataFields::COMPRESSIBLE_NAME :
             RdataFields::INCOMPRESSIBLE_NAME;
         name.toWire(buffer_);
         fields_.push_back(RdataFields::FieldSpec(field_type,
                                                  name.getLength()));
+        last_data_pos_ = buffer_.getLength();
     }
 
     virtual void clear() {
@@ -132,11 +107,31 @@ public:
         isc_throw(Unexpected,
                   "unexpected writeUint16At() for RdataFieldComposer");
     }
-    OutputBuffer buffer_;
     bool truncated_;
     size_t length_limit_;
     CompressMode mode_;
     vector<RdataFields::FieldSpec> fields_;
+    vector<RdataFields::FieldSpec>& getFields() {
+        extendData();
+        return (fields_);
+    }
+    // We use generict write* methods, with the exception of writeName.
+    // So new data can arriwe without us knowing it, this considers all new
+    // data to be just data and extends the fields to take it into account.
+    size_t last_data_pos_;
+    void extendData() {
+        // No news, return to work
+        if (buffer_.getLength() == last_data_pos_) {
+            return;
+        }
+        // The new bytes are just ordinary uninteresting data
+        if (fields_.empty() || fields_.back().type != RdataFields::DATA) {
+            fields_.push_back(RdataFields::FieldSpec(RdataFields::DATA, 0));
+        }
+        // We added this much data from last time
+        fields_.back().len += buffer_.getLength() - last_data_pos_;
+        last_data_pos_ = buffer_.getLength();
+    }
 };
 
 }
@@ -145,11 +140,11 @@ RdataFields::RdataFields(const Rdata& rdata) {
     OutputBuffer buffer(0);
     RdataFieldComposer field_composer(buffer);
     rdata.toWire(field_composer);
-    nfields_ = field_composer.fields_.size();
+    nfields_ = field_composer.getFields().size();
     data_length_ = field_composer.getLength();
     if (nfields_ > 0) {
         assert(data_length_ > 0);
-        detail_ = new RdataFieldsDetail(field_composer.fields_,
+        detail_ = new RdataFieldsDetail(field_composer.getFields(),
                                         static_cast<const uint8_t*>
                                         (field_composer.getData()),
                                         field_composer.getLength());

src/lib/dns/tests/rdatafields_unittest.cc

@@ -337,7 +337,7 @@ TEST_F(RdataFieldsTest, getFieldSpecWithBadFieldId) {
 // tested via other tests above.
 class DummyRdata : public Rdata {
 public:
-    enum Mode { CLEAR, SKIP, TRIM, WRITEUINT16AT };
+    enum Mode { CLEAR, SKIP, TRIM };
     explicit DummyRdata(Mode mode) : mode_(mode) {}
     DummyRdata(const DummyRdata& source) : Rdata(), mode_(source.mode_) {}
     virtual ~DummyRdata() {}
@@ -354,9 +354,6 @@ public:
         case TRIM:
             renderer.trim(2);
             break;
-        case WRITEUINT16AT:
-            renderer.writeUint16At(0, 0);
-            break;
         }
     }
     
@@ -373,7 +370,5 @@ TEST(RdataFieldComposerTest, unusedMethods) {
     EXPECT_THROW(RdataFields(DummyRdata(DummyRdata::CLEAR)), isc::Unexpected);
     EXPECT_THROW(RdataFields(DummyRdata(DummyRdata::SKIP)), isc::Unexpected);
     EXPECT_THROW(RdataFields(DummyRdata(DummyRdata::TRIM)), isc::Unexpected);
-    EXPECT_THROW(RdataFields(DummyRdata(DummyRdata::WRITEUINT16AT)),
-                 isc::Unexpected);
 }
 }