Browse Source

[2096] (Unrelated) MessageRenderer isn't copyable

Discovered the hard way - it can crash.

Small trick with just-in-time initialization of a pointer in benchmark
is used instead of copying it and relying on the fact that it can be
copied while it was not yet used.
Michal 'vorner' Vaner 12 years ago
parent
commit
305f3328e7

+ 11 - 4
src/lib/dns/benchmarks/message_renderer_bench.cc

@@ -35,21 +35,28 @@ template <typename T>
 class MessageRendererBenchMark {
 public:
     MessageRendererBenchMark(const vector<Name>& names) :
+        renderer_(NULL),
         names_(names)
     {}
+    MessageRendererBenchMark() {
+        delete renderer_;
+    }
     unsigned int run() {
-        renderer_.clear();
+        if (renderer_ == NULL) {
+            renderer_ = new T();
+        }
+        renderer_->clear();
         vector<Name>::const_iterator it = names_.begin();
         const vector<Name>::const_iterator it_end = names_.end();
         for (; it != it_end; ++it) {
-            renderer_.writeName(*it);
+            renderer_->writeName(*it);
         }
         // Make sure truncation didn't accidentally happen.
-        assert(!renderer_.isTruncated());
+        assert(!renderer_->isTruncated());
         return (names_.size());
     }
 private:
-    T renderer_;
+    T* renderer_; // It's pointer, so we won't need to copy it.
     const vector<Name>& names_;
 };
 

+ 10 - 3
src/lib/dns/benchmarks/rdatarender_bench.cc

@@ -44,18 +44,25 @@ public:
     RdataRenderBenchMark(const vector<T>& dataset) :
         dataset_(dataset)
     {}
+    RdataRenderBenchMark() {
+        delete renderer_;
+    }
     unsigned int run() {
+        if (renderer_ == NULL) {
+            renderer_ = new MessageRenderer();
+        }
         typename vector<T>::const_iterator data;
         typename vector<T>::const_iterator data_end = dataset_.end();
         for (data = dataset_.begin(); data != data_end; ++data) {
-            renderer_.clear();
-            (*data)->toWire(renderer_);
+            renderer_->clear();
+            (*data)->toWire(*renderer_);
         }
         return (dataset_.size());
     }
 private:
     const vector<T>& dataset_;
-    MessageRenderer renderer_;
+    // Just-in-time initialized pointer, so no copy
+    MessageRenderer* renderer_;
 };
 
 // This supplemental class emulates an RRset like class that internally

+ 4 - 1
src/lib/dns/messagerenderer.h

@@ -17,6 +17,8 @@
 
 #include <util/buffer.h>
 
+#include <boost/noncopyable.hpp>
+
 namespace isc {
 
 namespace dns {
@@ -346,7 +348,8 @@ public:
 /// end of the buffer at the time of construction.  However, if the
 /// pre-existing portion of the buffer contains DNS names, these names won't
 /// be considered for name compression.
-class MessageRenderer : public AbstractMessageRenderer {
+class MessageRenderer : public AbstractMessageRenderer,
+    public boost::noncopyable { // Can crash if copied
 public:
     using AbstractMessageRenderer::CASE_INSENSITIVE;
     using AbstractMessageRenderer::CASE_SENSITIVE;