svn commit: r944329 - in /qpid/trunk/qpid/cpp/src/qpid/framing: AMQFrame.cpp AMQFrame.h

[gs...@apache.org]

Author: gsim
Date: Fri May 14 16:08:33 2010
New Revision: 944329

URL: http://svn.apache.org/viewvc?rev=944329&view=rev
Log:
This reverts r736810. In my testing (admittedly limited) I did not observe any benefot from the optimisation. It causes a bug where concurrent calls to the non-const getBody() method can result in the encodedSize() method returning 0. This has implications e.g. for store modules where the message size may be incorrectly reported resulting in buffer overruns.

Modified:
    qpid/trunk/qpid/cpp/src/qpid/framing/AMQFrame.cpp
    qpid/trunk/qpid/cpp/src/qpid/framing/AMQFrame.h

Modified: qpid/trunk/qpid/cpp/src/qpid/framing/AMQFrame.cpp
URL: http://svn.apache.org/viewvc/qpid/trunk/qpid/cpp/src/qpid/framing/AMQFrame.cpp?rev=944329&r1=944328&r2=944329&view=diff
==============================================================================
--- qpid/trunk/qpid/cpp/src/qpid/framing/AMQFrame.cpp (original)
+++ qpid/trunk/qpid/cpp/src/qpid/framing/AMQFrame.cpp Fri May 14 16:08:33 2010
@@ -32,12 +32,7 @@
 namespace qpid {
 namespace framing {
 
-void AMQFrame::init() {
-    bof = eof = bos = eos = true;
-    subchannel=0;
-    channel=0;
-    encodedSizeCache = 0;
-}
+void AMQFrame::init() { bof = eof = bos = eos = true; subchannel=0; channel=0; }
 
 AMQFrame::AMQFrame(const boost::intrusive_ptr<AMQBody>& b) : body(b) { init(); }
 
@@ -45,28 +40,13 @@ AMQFrame::AMQFrame(const AMQBody& b) : b
 
 AMQFrame::~AMQFrame() {}
 
-AMQBody* AMQFrame::getBody() {
-    // Non-const AMQBody* may be used to modify the body.
-    encodedSizeCache = 0;
-    return body.get();
-}
-
-const AMQBody* AMQFrame::getBody() const {
-    return body.get();
-}
-
-void AMQFrame::setMethod(ClassId c, MethodId m) {
-    encodedSizeCache = 0;
-    body = MethodBodyFactory::create(c,m);
-}
+void AMQFrame::setMethod(ClassId c, MethodId m) { body = MethodBodyFactory::create(c,m); }
 
 uint32_t AMQFrame::encodedSize() const {
-    if (!encodedSizeCache) {
-        encodedSizeCache = frameOverhead() + body->encodedSize();
-        if (body->getMethod())
-            encodedSizeCache +=  sizeof(ClassId)+sizeof(MethodId);
-    }
-    return encodedSizeCache;
+    uint32_t size = frameOverhead() + body->encodedSize();
+    if (body->getMethod())
+        size +=  sizeof(ClassId)+sizeof(MethodId);
+    return size;
 }
 
 uint32_t AMQFrame::frameOverhead() {
@@ -102,13 +82,11 @@ void AMQFrame::encode(Buffer& buffer) co
 }
 
 bool AMQFrame::decode(Buffer& buffer)
-{
+{    
     if(buffer.available() < frameOverhead())
         return false;
     buffer.record();
 
-    encodedSizeCache = 0;
-    uint32_t start = buffer.getPosition();
     uint8_t  flags = buffer.getOctet();
     uint8_t framing_version = (flags & 0xc0) >> 6;
     if (framing_version != 0)
@@ -157,7 +135,7 @@ bool AMQFrame::decode(Buffer& buffer)
 	throw IllegalArgumentException(QPID_MSG("Invalid frame type " << type));
     }
     body->decode(buffer, body_size);
-    encodedSizeCache = buffer.getPosition() - start;
+
     return true;
 }
 

Modified: qpid/trunk/qpid/cpp/src/qpid/framing/AMQFrame.h
URL: http://svn.apache.org/viewvc/qpid/trunk/qpid/cpp/src/qpid/framing/AMQFrame.h?rev=944329&r1=944328&r2=944329&view=diff
==============================================================================
--- qpid/trunk/qpid/cpp/src/qpid/framing/AMQFrame.h (original)
+++ qpid/trunk/qpid/cpp/src/qpid/framing/AMQFrame.h Fri May 14 16:08:33 2010
@@ -43,8 +43,8 @@ class AMQFrame : public AMQDataBlock
     ChannelId getChannel() const { return channel; }
     void setChannel(ChannelId c) { channel = c; }
 
-    QPID_COMMON_EXTERN AMQBody* getBody();
-    QPID_COMMON_EXTERN const AMQBody* getBody() const;
+    AMQBody* getBody() { return body.get(); }
+    const AMQBody* getBody() const { return body.get(); }
 
     AMQMethodBody* getMethod() { return getBody() ? getBody()->getMethod() : 0; }
     const AMQMethodBody* getMethod() const { return getBody() ? getBody()->getMethod() : 0; }
@@ -102,7 +102,6 @@ class AMQFrame : public AMQDataBlock
     bool eof : 1;
     bool bos : 1;
     bool eos : 1;
-    mutable uint32_t encodedSizeCache;
 };
 
 QPID_COMMON_EXTERN std::ostream& operator<<(std::ostream&, const AMQFrame&);



---------------------------------------------------------------------
Apache Qpid - AMQP Messaging Implementation
Project:      http://qpid.apache.org
Use/Interact: mailto:commits-subscribe@qpid.apache.org