Added framing::BodyHolder:

 - Uniform holder for all body types, replaces MethodHolder.
 - Uses in_place constructors to avoid avoid body copy.

framing::AMQFrame:
 - Holds body in heap-allocated intrusive_ptr<BodyHolder>
 - Uses in_place constructors to avoid avoid body copy.

Removed/downgraded to TODO many redundant FIXME comments.


git-svn-id: https://svn.apache.org/repos/asf/incubator/qpid/trunk/qpid@597513 13f79535-47bb-0310-9956-ffa450edef68

15 files changed, 263 insertions, 319 deletions

diff --git a/cpp/src/qpid/framing/AMQBody.h b/cpp/src/qpid/framing/AMQBody.h
index 0b2718bef3..b05301bd05 100644
--- a/cpp/src/qpid/framing/AMQBody.h
+++ b/cpp/src/qpid/framing/AMQBody.h
@@ -22,7 +22,6 @@
  *
  */
 #include "qpid/framing/amqp_types.h"
-#include "qpid/shared_ptr.h"
 
 #include <ostream>
 
@@ -47,8 +46,6 @@ struct AMQBodyConstVisitor {
 class AMQBody
 {
   public:
-    typedef shared_ptr<AMQBody> shared_ptr;
-
     virtual ~AMQBody();
 
     virtual uint8_t type() const = 0;
diff --git a/cpp/src/qpid/framing/AMQFrame.cpp b/cpp/src/qpid/framing/AMQFrame.cpp
index 423af06173..3750f4a9a8 100644
--- a/cpp/src/qpid/framing/AMQFrame.cpp
+++ b/cpp/src/qpid/framing/AMQFrame.cpp
@@ -31,49 +31,16 @@
 namespace qpid {
 namespace framing {
 
-namespace {
-struct GetBodyVisitor : public NoBlankVisitor<AMQBody*> {
-    QPID_USING_NOBLANK(AMQBody*);
-    AMQBody* operator()(MethodHolder& t) const { return t.get(); }
-    template <class T> AMQBody* operator()(T& t) const { return &t; }
-};
-
-struct EncodeVisitor : public NoBlankVisitor<void> {
-    Buffer& buffer;
-    EncodeVisitor(Buffer& b) : buffer(b) {}
-    
-    QPID_USING_NOBLANK(void);
-    template <class T> void operator()(const T& t) const { return t.encode(buffer); }
-};
-
-struct SizeVisitor : public NoBlankVisitor<uint32_t> {
-    QPID_USING_NOBLANK(uint32_t);
-    template <class T> uint32_t operator()(const T& t) const { return t.size(); }
-};
-
-struct DecodeVisitor : public NoBlankVisitor<void> {
-    Buffer& buffer;
-    uint32_t size;
-    DecodeVisitor(Buffer& b, uint32_t s) : buffer(b), size(s) {}
-    QPID_USING_NOBLANK(void);
-    void operator()(MethodHolder& t) const { return t.decode(buffer); }
-    template <class T> void operator()(T& t) const { return t.decode(buffer, size); }
-};
+AMQFrame::~AMQFrame() {}
 
-}
+void AMQFrame::setBody(const AMQBody& b) { body = new BodyHolder(b); }
 
-AMQBody* AMQFrame::getBody() {
-    return boost::apply_visitor(GetBodyVisitor(), body);
-}
-
-const AMQBody* AMQFrame::getBody() const {
-    return boost::apply_visitor(GetBodyVisitor(), const_cast<Variant&>(body));
-}
+void AMQFrame::setMethod(ClassId c, MethodId m) { body = new BodyHolder(c,m); }
 
-// This is now misleadingly named as it is not the frame size as defined in the spec 
-// (as it also includes the end marker)
-uint32_t AMQFrame::size() const{
-    return frameOverhead() + boost::apply_visitor(SizeVisitor(), body);
+// This is now misleadingly named as it is not the frame size as
+// defined in the spec (as it also includes the end marker)
+uint32_t AMQFrame::size() const {
+    return frameOverhead() + body->size();
 }
 
 uint32_t AMQFrame::frameOverhead() {
@@ -90,7 +57,7 @@ void AMQFrame::encode(Buffer& buffer) const
     buffer.putOctet(0x0f & subchannel);
     buffer.putShort(channel);    
     buffer.putLong(0);
-    boost::apply_visitor(EncodeVisitor(buffer), body);
+    body->encode(buffer);
     buffer.putOctet(0xCE);
 }
 
@@ -119,45 +86,34 @@ bool AMQFrame::decode(Buffer& buffer)
     (void) buffer.getLong(); // reserved2
     
     // Verify that the protocol header meets current spec
-    // TODO: should we check reserved2 against zero as well? - the spec isn't clear
+    // TODO: should we check reserved2 against zero as well? - the
+    // spec isn't clear
     if ((flags & 0x30) != 0 || reserved1 != 0 || (field1 & 0xf0) != 0)
         throw SyntaxErrorException(QPID_MSG("Reserved bits not zero"));
 
-    // TODO: should no longer care about body size and only pass up B,E,b,e flags
+    // TODO: should no longer care about body size and only pass up
+    // B,E,b,e flags
     uint16_t body_size = frame_size + 1 - frameOverhead(); 
     if (buffer.available() < body_size+1u){
         buffer.restore();
         return false;
     }
-    decodeBody(buffer, body_size, type);
-
+    body = new BodyHolder();
+    body->decode(type,buffer, body_size);
     uint8_t end = buffer.getOctet();
     if (end != 0xCE)
         throw SyntaxErrorException(QPID_MSG("Frame end not found"));
     return true;
 }
 
-void AMQFrame::decodeBody(Buffer& buffer, uint32_t size, uint8_t type)
-{    
-    switch(type)
-    {
-      case METHOD_BODY: body = MethodHolder(); break;
-      case HEADER_BODY: body = AMQHeaderBody();	break;
-      case CONTENT_BODY: body = AMQContentBody(); break;
-      case HEARTBEAT_BODY: body = AMQHeartbeatBody(); break;
-
-      default:
-	throw SyntaxErrorException(QPID_MSG("Invalid frame type " << type));
-    }
-    boost::apply_visitor(DecodeVisitor(buffer,size), body);
-}
-
 std::ostream& operator<<(std::ostream& out, const AMQFrame& f)
 {
-    return out << "Frame[" 
-               << (f.getBof() ? "B" : "") << (f.getEof() ? "E" : "") << (f.getBos() ? "b" : "") << (f.getEos() ? "e" : "") << "; "
-               << "channel=" << f.getChannel() << "; " << *f.getBody()
-               << "]";
+    return
+        out << "Frame[" 
+            << (f.getBof() ? "B" : "") << (f.getEof() ? "E" : "")
+            << (f.getBos() ? "b" : "") << (f.getEos() ? "e" : "") << "; "
+            << "channel=" << f.getChannel() << "; " << *f.getBody()
+            << "]";
 }
 
 
diff --git a/cpp/src/qpid/framing/AMQFrame.h b/cpp/src/qpid/framing/AMQFrame.h
index 392b8c0c01..1c65988b3d 100644
--- a/cpp/src/qpid/framing/AMQFrame.h
+++ b/cpp/src/qpid/framing/AMQFrame.h
@@ -25,46 +25,48 @@
 #include "AMQHeaderBody.h"
 #include "AMQContentBody.h"
 #include "AMQHeartbeatBody.h"
-#include "MethodHolder.h"
 #include "ProtocolVersion.h"
+#include "BodyHolder.h"
 
 #include <boost/cast.hpp>
-#include <boost/variant.hpp>
 
 namespace qpid {
 namespace framing {
-	
+
+class BodyHolder;
+
 class AMQFrame : public AMQDataBlock
 {
   public:
-    AMQFrame() : bof(true), eof(true), bos(true), eos(true), subchannel(0), channel(0) {}
+    AMQFrame(intrusive_ptr<BodyHolder> b=0) : body(b) { init(); }
+    AMQFrame(const AMQBody& b) { setBody(b); init(); }
+    ~AMQFrame();
 
-    /** Construct a frame with a copy of b */
-    AMQFrame(ChannelId c, const AMQBody* b) : bof(true), eof(true), bos(true), eos(true), subchannel(0), channel(c) {
-        setBody(*b);
-    }
-    
-    AMQFrame(ChannelId c, const AMQBody& b) : bof(true), eof(true), bos(true), eos(true), subchannel(0), channel(c) {
-        setBody(b);
+    template <class InPlace>
+    AMQFrame(const InPlace& ip, typename EnableInPlace<InPlace>::type* =0) {
+        init(); setBody(ip);
     }
-    
-    AMQFrame(const AMQBody& b) : bof(true), eof(true), bos(true), eos(true), subchannel(0), channel(0) {
-        setBody(b);
-    }
-    
+
     ChannelId getChannel() const { return channel; }
     void setChannel(ChannelId c) { channel = c; }
 
-    AMQBody* getBody();
-    const AMQBody* getBody() const;
+    intrusive_ptr<BodyHolder> getHolder() { return body; }
+    
+    AMQBody* getBody() { return body ? body->get() : 0; }
+    const AMQBody* getBody() const { return body ? body->get() : 0; }
 
     AMQMethodBody* getMethod() { return getBody()->getMethod(); }
     const AMQMethodBody* getMethod() const { return getBody()->getMethod(); }
 
-    /** Copy a body instance to the frame */
-    void setBody(const AMQBody& b) { CopyVisitor cv(*this); b.accept(cv); }
+    void setBody(const AMQBody& b);
+
+    template <class InPlace>
+    typename EnableInPlace<InPlace>::type setBody(const InPlace& ip) {
+        body = new BodyHolder(ip);
+    }
+
+    void setMethod(ClassId c, MethodId m);
 
-    /** Convenience template to cast the body to an expected type. */
     template <class T> T* castBody() {
         return boost::polymorphic_downcast<T*>(getBody());
     }
@@ -73,8 +75,6 @@ class AMQFrame : public AMQDataBlock
         return boost::polymorphic_downcast<const T*>(getBody());
     }
 
-    bool empty() { return boost::get<boost::blank>(&body); }
-
     void encode(Buffer& buffer) const; 
     bool decode(Buffer& buffer); 
     uint32_t size() const;
@@ -92,33 +92,15 @@ class AMQFrame : public AMQDataBlock
     static uint32_t frameOverhead();
 
   private:
-    struct CopyVisitor : public AMQBodyConstVisitor {
-        AMQFrame& frame;
-        CopyVisitor(AMQFrame& f) : frame(f) {}
-        void visit(const AMQHeaderBody& x) { frame.body=x; }
-        void visit(const AMQContentBody& x) { frame.body=x; }
-        void visit(const AMQHeartbeatBody& x) { frame.body=x; }
-        void visit(const AMQMethodBody& x) { frame.body=MethodHolder(x); }
-    };
-    friend struct CopyVisitor;
-
-    typedef boost::variant<boost::blank,
-                           AMQHeaderBody,
-                           AMQContentBody,
-                           AMQHeartbeatBody,
-                           MethodHolder> Variant;
-
-    void visit(AMQHeaderBody& x) { body=x; }
-
-    void decodeBody(Buffer& buffer, uint32_t size, uint8_t type);
-
-    bool bof;
-    bool eof;
-    bool bos;
-    bool eos;
-    uint8_t subchannel;
-    uint16_t channel;
-    Variant body;
+    void init() { bof = eof = bos = eos = true; subchannel=0; channel=0; }
+
+    intrusive_ptr<BodyHolder> body;
+    uint16_t channel : 16;
+    uint8_t subchannel : 8;
+    bool bof : 1;
+    bool eof : 1;
+    bool bos : 1;
+    bool eos : 1;
 };
 
 std::ostream& operator<<(std::ostream&, const AMQFrame&);
diff --git a/cpp/src/qpid/framing/AMQMethodBody.h b/cpp/src/qpid/framing/AMQMethodBody.h
index 9f64fd1690..da28ee3aa9 100644
--- a/cpp/src/qpid/framing/AMQMethodBody.h
+++ b/cpp/src/qpid/framing/AMQMethodBody.h
@@ -40,8 +40,6 @@ class MethodBodyConstVisitor;
 class AMQMethodBody : public AMQBody {
   public:
     AMQMethodBody() {}
-    AMQMethodBody(uint8_t, uint8_t) {}
-    
     virtual ~AMQMethodBody();
 
     virtual void accept(MethodBodyConstVisitor&) const = 0;
diff --git a/cpp/src/qpid/framing/Blob.h b/cpp/src/qpid/framing/Blob.h
index 9d0c33dee6..344e4ac4db 100644
--- a/cpp/src/qpid/framing/Blob.h
+++ b/cpp/src/qpid/framing/Blob.h
@@ -25,38 +25,45 @@
 #include <boost/aligned_storage.hpp>
 #include <boost/checked_delete.hpp>
 #include <boost/utility/typed_in_place_factory.hpp>
+#include <boost/type_traits/is_base_and_derived.hpp>
+#include <boost/utility/enable_if.hpp>
 
 #include <new>
 
 #include <assert.h>
 
 
-namespace boost {
+namespace qpid {
+namespace framing {
 
-/**
- * 0-arg typed_in_place_factory constructor and in_place() override.
- * 
- * Boost doesn't provide the 0 arg version since it assumes
- * in_place_factory will be used when there is no default ctor.
- */
+using boost::in_place;          
+using boost::typed_in_place_factory_base;
+
+/** 0-arg typed_in_place_factory, missing in boost. */
 template <class T>
-class typed_in_place_factory0 : public typed_in_place_factory_base {
-  public:
+struct typed_in_place_factory0 : public typed_in_place_factory_base {
     typedef T value_type ; 
     void apply ( void* address ) const { new (address) T(); }
 };
 
+/** 0-arg in_place<T>() function, missing from boost. */
 template<class T>
 typed_in_place_factory0<T> in_place() { return typed_in_place_factory0<T>(); }
 
-} // namespace boost
-
-
-namespace qpid {
-namespace framing {
-
-using boost::in_place;
-
+template <class T, class R=void>
+struct EnableInPlace
+    : public boost::enable_if<boost::is_base_and_derived<
+                                  typed_in_place_factory_base, T>,
+                              R>
+{};
+       
+template <class T, class R=void>
+struct DisableInPlace
+    : public boost::disable_if<boost::is_base_and_derived<
+                                   typed_in_place_factory_base, T>,
+                               R>
+{};
+       
 template <class T> struct BlobHelper {
     static void destroy(void* ptr) { static_cast<T*>(ptr)->~T(); }
     static void copy(void* dest, const void* src) {
@@ -108,11 +115,10 @@ class Blob
         basePtr=0;
     }
 
-    template<class TypedInPlaceFactory>
-    void construct (const TypedInPlaceFactory& factory,
-                    const boost::typed_in_place_factory_base* )
+    template<class Factory>
+    typename EnableInPlace<Factory>::type apply(const Factory& factory)
     {
-        typedef typename TypedInPlaceFactory::value_type T;
+        typedef typename Factory::value_type T;
         assert(empty());
         factory.apply(store.address());
         setType<T>();
@@ -135,28 +141,31 @@ class Blob
     Blob(const Blob& b) { initialize(); assign(b); }
 
     /** @see construct() */
-    template<class Expr>
-    Blob( const Expr & expr ) { initialize(); construct(expr,&expr); }
+    template<class InPlace>
+    Blob(const InPlace & expr, typename EnableInPlace<InPlace>::type* =0) {
+        initialize(); apply(expr);
+    }
 
     ~Blob() { clear(); }
 
-    /** Assign a blob */
+    /** Assign from another blob. */
     Blob& operator=(const Blob& b) {
         clear();
         assign(b);
         return *this;
     }
 
-    /** Construcct an object in the blob. Destroyes the previous object.
-     *@param expr an expresion of the form: in_place<T>(x,y,z)
-     * will construct an object using the constructor T(x,y,z)
-     */
-    template<class Expr> void
-    construct(const Expr& expr) { clear(); construct(expr,&expr); }
+    /** Assign from an in_place constructor expression. */
+    template<class InPlace>
+    typename EnableInPlace<InPlace,Blob&>::type operator=(const InPlace& expr) {
+        clear(); apply(expr); return *this;
+    }
 
-    /** Copy construct an instance of T into the Blob. */
+    /** Assign from an object of type T. */
     template <class T>
-    Blob& operator=(const T& x) { clear(); construct(in_place<T>(x)); return *this; }
+    typename DisableInPlace<T, Blob&>::type operator=(const T& x) {
+        clear(); apply(in_place<T>(x)); return *this;
+    }
 
     /** Get pointer to blob contents, returns 0 if empty. */
     BaseType* get() { return  basePtr; }
diff --git a/cpp/src/qpid/framing/BodyHolder.cpp b/cpp/src/qpid/framing/BodyHolder.cpp
new file mode 100644
index 0000000000..f66f29d36a
--- /dev/null
+++ b/cpp/src/qpid/framing/BodyHolder.cpp
@@ -0,0 +1,75 @@
+/*
+ *
+ * Licensed to the Apache Software Foundation (ASF) under one
+ * or more contributor license agreements.  See the NOTICE file
+ * distributed with this work for additional information
+ * regarding copyright ownership.  The ASF licenses this file
+ * to you under the Apache License, Version 2.0 (the
+ * "License"); you may not use this file except in compliance
+ * with the License.  You may obtain a copy of the License at
+ * 
+ *   http://www.apache.org/licenses/LICENSE-2.0
+ * 
+ * Unless required by applicable law or agreed to in writing,
+ * software distributed under the License is distributed on an
+ * "AS IS" BASIS, WITHOUT WARRANTIES OR CONDITIONS OF ANY
+ * KIND, either express or implied.  See the License for the
+ * specific language governing permissions and limitations
+ * under the License.
+ *
+ */
+#include "BodyHolder.h"
+#include "AMQMethodBody.h"
+#include "AMQHeaderBody.h"
+#include "AMQContentBody.h"
+#include "AMQHeartbeatBody.h"
+#include "Buffer.h"
+#include "qpid/framing/reply_exceptions.h"
+
+namespace qpid {
+namespace framing {
+
+
+// BodyHolder::operator=(const AMQBody&)  is defined
+// in generated file BodyHolder_gen.cpp
+
+
+void BodyHolder::encode(Buffer& b) const {
+    const AMQMethodBody* method=getMethod();
+    if (method) {
+        b.putOctet(method->amqpClassId());
+        b.putOctet(method->amqpMethodId());
+        method->encode(b);
+    }
+    else
+        get()->encode(b);
+}
+
+void BodyHolder::decode(uint8_t type, Buffer& buffer, uint32_t size) {
+    switch(type)
+    {
+      case METHOD_BODY: {
+          ClassId c = buffer.getOctet();
+          MethodId m = buffer.getOctet();
+          setMethod(c, m);
+          break;
+      }
+      case HEADER_BODY: *this=in_place<AMQHeaderBody>(); break;
+      case CONTENT_BODY: *this=in_place<AMQContentBody>(); break;
+      case HEARTBEAT_BODY: *this=in_place<AMQHeartbeatBody>(); break;
+      default:
+	throw SyntaxErrorException(QPID_MSG("Invalid frame type " << type));
+    }
+    get()->decode(buffer, size);
+}
+
+uint32_t BodyHolder::size() const {
+    const AMQMethodBody* method=getMethod();
+    if (method) 
+        return sizeof(ClassId)+sizeof(MethodId)+method->size();
+    else
+        return get()->size();
+}
+
+}} // namespace qpid::framing
+
diff --git a/cpp/src/qpid/framing/BodyHolder.h b/cpp/src/qpid/framing/BodyHolder.h
new file mode 100644
index 0000000000..65029e5675
--- /dev/null
+++ b/cpp/src/qpid/framing/BodyHolder.h
@@ -0,0 +1,88 @@
+#ifndef QPID_FRAMING_BODYHOLDER_H
+#define QPID_FRAMING_BODYHOLDER_H
+
+/*
+ *
+ * Licensed to the Apache Software Foundation (ASF) under one
+ * or more contributor license agreements.  See the NOTICE file
+ * distributed with this work for additional information
+ * regarding copyright ownership.  The ASF licenses this file
+ * to you under the Apache License, Version 2.0 (the
+ * "License"); you may not use this file except in compliance
+ * with the License.  You may obtain a copy of the License at
+ * 
+ *   http://www.apache.org/licenses/LICENSE-2.0
+ * 
+ * Unless required by applicable law or agreed to in writing,
+ * software distributed under the License is distributed on an
+ * "AS IS" BASIS, WITHOUT WARRANTIES OR CONDITIONS OF ANY
+ * KIND, either express or implied.  See the License for the
+ * specific language governing permissions and limitations
+ * under the License.
+ *
+ */
+
+#include "qpid/framing/AMQBody.h"
+#include "qpid/framing/Blob.h"
+#include "qpid/framing/MaxMethodBodySize.h" // Generated file.
+#include "qpid/framing/amqp_types.h"
+#include "qpid/RefCounted.h"
+
+
+namespace qpid {
+namespace framing {
+
+class AMQMethodBody;
+class AMQBody;
+class Buffer;
+
+/**
+ * Holder for arbitrary frame body.
+ */
+class BodyHolder : public RefCounted
+{
+  public:
+    // default copy, assign dtor ok.
+    BodyHolder() {}
+    BodyHolder(const AMQBody& b) { setBody(b); }
+    BodyHolder(ClassId c, MethodId m) { setMethod(c,m); }
+
+    /** Construct from an in_place constructor expression */
+    template <class InPlace>
+    BodyHolder(const InPlace& ip, typename EnableInPlace<InPlace>::type* =0)
+        : blob(ip) {}
+
+    void setBody(const AMQBody& b);
+
+    /** Assign from an in_place constructor expression */
+    template <class InPlace>
+    typename EnableInPlace<InPlace,BodyHolder&>::type
+    operator=(const InPlace& ip) { blob=ip; return *this; }
+
+    /** Assign by copying. */
+    template <class T>
+    typename DisableInPlace<T,BodyHolder&>::type operator=(const T& x)
+    { blob=in_place<T>(x); return *this; }
+
+    /** Set to method with ClassId c, MethodId m. */
+    void setMethod(ClassId c, MethodId m);
+
+    void encode(Buffer&) const;
+    void decode(uint8_t frameType, Buffer&, uint32_t=0);
+    uint32_t size() const;
+
+    /** Return body pointer or 0 if empty. */
+    AMQBody* get() { return blob.get(); }
+    const AMQBody* get() const { return blob.get(); }
+
+    /** Return method pointer or 0 if not a method. */
+    AMQMethodBody* getMethod() { return get()->getMethod(); }
+    const AMQMethodBody* getMethod() const { return get()->getMethod(); }
+
+  private:
+    Blob<MAX_METHOD_BODY_SIZE, AMQBody> blob;
+};
+
+}} // namespace qpid::framing
+
+#endif  /*!QPID_FRAMING_BODYHOLDER_H*/
diff --git a/cpp/src/qpid/framing/ChannelAdapter.cpp b/cpp/src/qpid/framing/ChannelAdapter.cpp
index 8c1a4e1e9e..a1e49a1904 100644
--- a/cpp/src/qpid/framing/ChannelAdapter.cpp
+++ b/cpp/src/qpid/framing/ChannelAdapter.cpp
@@ -43,7 +43,8 @@ void ChannelAdapter::init(ChannelId i, FrameHandler& out, ProtocolVersion v)
 void ChannelAdapter::send(const AMQBody& body)
 {
     assertChannelOpen();
-    AMQFrame frame(getId(), body);
+    AMQFrame frame(body);
+    frame.setChannel(getId());
     handlers.out(frame);
 }
 
diff --git a/cpp/src/qpid/framing/FrameDefaultVisitor.h b/cpp/src/qpid/framing/FrameDefaultVisitor.h
index 93a5204308..f695414977 100644
--- a/cpp/src/qpid/framing/FrameDefaultVisitor.h
+++ b/cpp/src/qpid/framing/FrameDefaultVisitor.h
@@ -24,8 +24,6 @@
 #include "qpid/framing/MethodBodyDefaultVisitor.h"
 #include "qpid/framing/AMQBody.h"
 #include "qpid/framing/AMQMethodBody.h"
-#include "qpid/framing/AMQFrame.h"
-#include "qpid/framing/FrameHandler.h"
 
 namespace qpid {
 namespace framing {
diff --git a/cpp/src/qpid/framing/InputHandler.h b/cpp/src/qpid/framing/InputHandler.h
index 99e4e774e1..3a6d786a24 100644
--- a/cpp/src/qpid/framing/InputHandler.h
+++ b/cpp/src/qpid/framing/InputHandler.h
@@ -27,7 +27,7 @@
 namespace qpid {
 namespace framing {
 
-// FIXME aconway 2007-08-29: Eliminate, replace with FrameHandler.
+// TODO aconway 2007-08-29: Eliminate, replace with FrameHandler.
 class InputHandler : public FrameHandler {
   public:
     virtual ~InputHandler() {}
diff --git a/cpp/src/qpid/framing/MethodHolder.cpp b/cpp/src/qpid/framing/MethodHolder.cpp
deleted file mode 100644
index 78dd2bdcaf..0000000000
--- a/cpp/src/qpid/framing/MethodHolder.cpp
+++ /dev/null
@@ -1,65 +0,0 @@
-/*
- *
- * Licensed to the Apache Software Foundation (ASF) under one
- * or more contributor license agreements.  See the NOTICE file
- * distributed with this work for additional information
- * regarding copyright ownership.  The ASF licenses this file
- * to you under the Apache License, Version 2.0 (the
- * "License"); you may not use this file except in compliance
- * with the License.  You may obtain a copy of the License at
- * 
- *   http://www.apache.org/licenses/LICENSE-2.0
- * 
- * Unless required by applicable law or agreed to in writing,
- * software distributed under the License is distributed on an
- * "AS IS" BASIS, WITHOUT WARRANTIES OR CONDITIONS OF ANY
- * KIND, either express or implied.  See the License for the
- * specific language governing permissions and limitations
- * under the License.
- *
- */
-
-#include "MethodHolder.h"
-#include "qpid/framing/AMQMethodBody.h"
-#include "qpid/framing/Buffer.h"
-
-// Note: MethodHolder::construct is and operator= are code-generated
-// in file MethodHolder_construct.cpp.
-
-using namespace boost;
-
-namespace qpid {
-namespace framing {
-
-AMQMethodBody* MethodHolder::get() {
-    return blob.get();
-}
-
-const AMQMethodBody* MethodHolder::get() const {
-    return blob.get();
-}
-
-void MethodHolder::encode(Buffer& b) const {
-    const AMQMethodBody* body = get();
-    b.putOctet(body->amqpClassId());
-    b.putOctet(body->amqpMethodId());
-    body->encode(b);
-}
-
-void MethodHolder::decode(Buffer& b) {
-    ClassId c=b.getOctet();
-    MethodId m=b.getOctet();
-    construct(c,m);
-    get()->decode(b);
-}
-
-uint32_t  MethodHolder::size() const {
-    return sizeof(ClassId)+sizeof(MethodId)+get()->size();
-}
-
-std::ostream& operator<<(std::ostream& out, const MethodHolder& h) {
-    h.get()->print(out);
-    return out;
-}
-
-}} // namespace qpid::framing
diff --git a/cpp/src/qpid/framing/MethodHolder.h b/cpp/src/qpid/framing/MethodHolder.h
deleted file mode 100644
index 5324b34ccd..0000000000
--- a/cpp/src/qpid/framing/MethodHolder.h
+++ /dev/null
@@ -1,96 +0,0 @@
-#ifndef QPID_FRAMING_METHODHOLDER_H
-#define QPID_FRAMING_METHODHOLDER_H
-
-/*
- *
- * Licensed to the Apache Software Foundation (ASF) under one
- * or more contributor license agreements.  See the NOTICE file
- * distributed with this work for additional information
- * regarding copyright ownership.  The ASF licenses this file
- * to you under the Apache License, Version 2.0 (the
- * "License"); you may not use this file except in compliance
- * with the License.  You may obtain a copy of the License at
- * 
- *   http://www.apache.org/licenses/LICENSE-2.0
- * 
- * Unless required by applicable law or agreed to in writing,
- * software distributed under the License is distributed on an
- * "AS IS" BASIS, WITHOUT WARRANTIES OR CONDITIONS OF ANY
- * KIND, either express or implied.  See the License for the
- * specific language governing permissions and limitations
- * under the License.
- *
- */
-
-#include "qpid/framing/AMQBody.h"
-#include "qpid/framing/amqp_types.h"
-#include "qpid/framing/amqp_types.h"
-#include "qpid/framing/Blob.h"
-#include "qpid/framing/MethodHolderMaxSize.h" // Generated file.
-
-#include <boost/type_traits/is_base_and_derived.hpp>
-#include <boost/utility/enable_if.hpp>
-
-#include <utility>
-
-namespace qpid {
-namespace framing {
-
-class AMQMethodBody;
-class AMQBody;
-class Buffer;
-
-class MethodHolder;
-std::ostream& operator<<(std::ostream& out, const MethodHolder& h);
-
-/**
- * Holder for arbitrary method body.
- */
-// TODO aconway 2007-08-14: Fix up naming, this class should really be
-// called AMQMethodBody and use a different name for the root of
-// the concrete method body tree, which should not inherit AMQBody.
-// 
-class MethodHolder
-{
-    template <class T> struct EnableIfMethod:
-        public boost::enable_if<boost::is_base_and_derived<AMQMethodBody,T>,T>
-    {};
-
-    template <class T> EnableIfMethod<T>& assertMethod(T& t) { return t; }
-        
-  public:
-    MethodHolder() {}
-    MethodHolder(ClassId& c, MethodId& m) { construct(c,m); }
-
-    /** Construct with a copy of a method body. */
-    MethodHolder(const AMQMethodBody& m) { *this = m; }
-
-    /** Copy method body into holder. */
-    MethodHolder& operator=(const AMQMethodBody&);
-
-    /** Construct the method body corresponding to class/method id */
-    void construct(ClassId c, MethodId m);
-
-    uint8_t type() const { return 1; }
-    void encode(Buffer&) const;
-    void decode(Buffer&);
-    uint32_t size() const;
-
-    /** Return method pointer or 0 if empty. */
-    AMQMethodBody* get();
-    const AMQMethodBody* get() const;
-
-    /** True if no method has been set */
-    bool empty() const { return blob.empty(); }
-
-  private:
-    Blob<MAX_METHODBODY_SIZE, AMQMethodBody> blob;
-    class CopyVisitor;
-  friend struct CopyVisitor;
-};
-
-
-
-}} // namespace qpid::framing
-
-#endif  /*!QPID_FRAMING_METHODHOLDER_H*/
diff --git a/cpp/src/qpid/framing/OutputHandler.h b/cpp/src/qpid/framing/OutputHandler.h
index 925ff88b12..6f4b27fb72 100644
--- a/cpp/src/qpid/framing/OutputHandler.h
+++ b/cpp/src/qpid/framing/OutputHandler.h
@@ -27,7 +27,7 @@
 namespace qpid {
 namespace framing {
 
-// FIXME aconway 2007-08-29: Replace with FrameHandler.
+// TODO aconway 2007-08-29: Replace with FrameHandler.
 class OutputHandler : public FrameHandler {
   public:
     virtual ~OutputHandler() {}
diff --git a/cpp/src/qpid/framing/SendContent.cpp b/cpp/src/qpid/framing/SendContent.cpp
index 57ac58038b..85b9b995ad 100644
--- a/cpp/src/qpid/framing/SendContent.cpp
+++ b/cpp/src/qpid/framing/SendContent.cpp
@@ -51,7 +51,8 @@ void qpid::framing::SendContent::operator()(const AMQFrame& f)
 
 void qpid::framing::SendContent::sendFragment(const AMQContentBody& body, uint32_t offset, uint16_t size, bool first, bool last) const
 {
-    AMQFrame fragment(0, AMQContentBody(body.getData().substr(offset, size)));
+    AMQFrame fragment(in_place<AMQContentBody>(
+                          body.getData().substr(offset, size)));
     setFlags(fragment, first, last);
     handler.handle(fragment);
 }
diff --git a/cpp/src/qpid/framing/variant.h b/cpp/src/qpid/framing/variant.h
index 1fe81f8f67..ceaed2c529 100644
--- a/cpp/src/qpid/framing/variant.h
+++ b/cpp/src/qpid/framing/variant.h
@@ -32,7 +32,7 @@ class Buffer;
 
 /** boost::static_visitor that throws exception if variant contains blank.
  * Sublclasses need to have a using() declaration, can be generated
- * with QPID_USING_BLANK_THROW(R)
+ * with QPID_USING_NOBLANK(R)
  */
 template <class R=void>
 struct NoBlankVisitor : public boost::static_visitor<R> {