BIND 10 trac404, updated. a125177e363f66375aca5af53649b8e369af58e6 [trac404] Remove duplicate code

BIND 10 source code commits bind10-changes at lists.isc.org
Wed Apr 6 08:44:15 UTC 2011


The branch, trac404 has been updated
       via  a125177e363f66375aca5af53649b8e369af58e6 (commit)
      from  69e7fd6b5782434c809f90d8060e148318ba3c7e (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 a125177e363f66375aca5af53649b8e369af58e6
Author: Michal 'vorner' Vaner <michal.vaner at nic.cz>
Date:   Wed Apr 6 10:38:50 2011 +0200

    [trac404] Remove duplicate code

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

Summary of changes:
 src/lib/dns/messagerenderer.cc            |   59 +++++----------------------
 src/lib/dns/messagerenderer.h             |   48 +++++++++++++++--------
 src/lib/dns/rdatafields.cc                |   61 +++++++++++++---------------
 src/lib/dns/tests/rdatafields_unittest.cc |    7 +---
 4 files changed, 72 insertions(+), 103 deletions(-)

-----------------------------------------------------------------------
diff --git a/src/lib/dns/messagerenderer.cc b/src/lib/dns/messagerenderer.cc
index 212411a..398f0be 100644
--- a/src/lib/dns/messagerenderer.cc
+++ b/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));
diff --git a/src/lib/dns/messagerenderer.h b/src/lib/dns/messagerenderer.h
index b698d22..90831ea 100644
--- a/src/lib/dns/messagerenderer.h
+++ b/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;
diff --git a/src/lib/dns/rdatafields.cc b/src/lib/dns/rdatafields.cc
index b387eac..49634c6 100644
--- a/src/lib/dns/rdatafields.cc
+++ b/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());
diff --git a/src/lib/dns/tests/rdatafields_unittest.cc b/src/lib/dns/tests/rdatafields_unittest.cc
index 7c17b54..7ab7041 100644
--- a/src/lib/dns/tests/rdatafields_unittest.cc
+++ b/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);
 }
 }




More information about the bind10-changes mailing list