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.

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

2 files changed, 10 insertions, 33 deletions

diff --git a/cpp/src/qpid/framing/AMQFrame.cpp b/cpp/src/qpid/framing/AMQFrame.cpp
index d863970ece..cd60cd971f 100644
--- a/cpp/src/qpid/framing/AMQFrame.cpp
+++ b/cpp/src/qpid/framing/AMQFrame.cpp
@@ -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) : body(b.clone()) { init(); }
 
 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) const
 }
 
 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;
 }
 
diff --git a/cpp/src/qpid/framing/AMQFrame.h b/cpp/src/qpid/framing/AMQFrame.h
index 29e368b671..d7b04f0f65 100644
--- a/cpp/src/qpid/framing/AMQFrame.h
+++ b/cpp/src/qpid/framing/AMQFrame.h
@@ -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&);