Revert "Checking structure sizes before reading them from memory to avoid overflowing the buffer's stream."

This reverts commit 6bc22e8ef1ea70a1b58409aa21254358c50f149a.

git-svn-id: http://skia.googlecode.com/svn/trunk@12124 2bbb7eff-a529-9590-31e7-b0007b416f81

8 files changed, 34 insertions, 92 deletions

diff --git a/src/core/SkBuffer.cpp b/src/core/SkBuffer.cpp
index 32a8011ac7..915264d957 100644
--- a/src/core/SkBuffer.cpp
+++ b/src/core/SkBuffer.cpp
@@ -34,13 +34,6 @@ size_t SkRBuffer::skipToAlign4()
     return n;
 }
 
-void SkRBufferWithSizeCheck::read(void* buffer, size_t size) {
-    fError = fError || (fPos + size > fStop);
-    if (!fError && (size > 0)) {
-        readNoSizeCheck(buffer, size);
-    }
-}
-
 void* SkWBuffer::skip(size_t size)
 {
     void* result = fPos;
diff --git a/src/core/SkBuffer.h b/src/core/SkBuffer.h
index 1a4c6c281c..9633389907 100644
--- a/src/core/SkBuffer.h
+++ b/src/core/SkBuffer.h
@@ -56,7 +56,7 @@ public:
     /** Read the specified number of bytes from the data pointer. If buffer is not
         null, copy those bytes into buffer.
     */
-    virtual void read(void* buffer, size_t size) {
+    void read(void* buffer, size_t size) {
         if (size) {
             this->readNoSizeCheck(buffer, size);
         }
@@ -74,7 +74,7 @@ public:
     uint8_t     readU8() { uint8_t x; read(&x, 1); return x; }
     bool        readBool() { return this->readU8() != 0; }
 
-protected:
+private:
     void    readNoSizeCheck(void* buffer, size_t size);
 
     const char* fData;
@@ -82,28 +82,6 @@ protected:
     const char* fStop;
 };
 
-/** \class SkRBufferWithSizeCheck
-
-    Same as SkRBuffer, except that a size check is performed before the read operation and an
-    error is set if the read operation is attempting to read past the end of the data.
-*/
-class SkRBufferWithSizeCheck : public SkRBuffer {
-public:
-    SkRBufferWithSizeCheck(const void* data, size_t size) : SkRBuffer(data, size), fError(false) {}
-
-    /** Read the specified number of bytes from the data pointer. If buffer is not
-        null and the number of bytes to read does not overflow this object's data,
-        copy those bytes into buffer.
-    */
-    virtual void read(void* buffer, size_t size) SK_OVERRIDE;
-
-    /** Returns whether or not a read operation attempted to read past the end of the data.
-    */
-    bool isValid() const { return !fError; }
-private:
-    bool fError;
-};
-
 /** \class SkWBuffer
 
     Light weight class for writing data to a memory block.
diff --git a/src/core/SkMatrix.cpp b/src/core/SkMatrix.cpp
index cd7bcea176..5bcb35b298 100644
--- a/src/core/SkMatrix.cpp
+++ b/src/core/SkMatrix.cpp
@@ -1921,25 +1921,20 @@ const SkMatrix& SkMatrix::InvalidMatrix() {
 
 ///////////////////////////////////////////////////////////////////////////////
 
-size_t SkMatrix::writeToMemory(void* buffer) const {
+uint32_t SkMatrix::writeToMemory(void* buffer) const {
     // TODO write less for simple matrices
-    static const size_t sizeInMemory = 9 * sizeof(SkScalar);
     if (buffer) {
-        memcpy(buffer, fMat, sizeInMemory);
+        memcpy(buffer, fMat, 9 * sizeof(SkScalar));
     }
-    return sizeInMemory;
+    return 9 * sizeof(SkScalar);
 }
 
-size_t SkMatrix::readFromMemory(const void* buffer, size_t length) {
-    static const size_t sizeInMemory = 9 * sizeof(SkScalar);
-    if (length < sizeInMemory) {
-        return 0;
-    }
+uint32_t SkMatrix::readFromMemory(const void* buffer) {
     if (buffer) {
-        memcpy(fMat, buffer, sizeInMemory);
+        memcpy(fMat, buffer, 9 * sizeof(SkScalar));
         this->setTypeMask(kUnknown_Mask);
     }
-    return sizeInMemory;
+    return 9 * sizeof(SkScalar);
 }
 
 #ifdef SK_DEVELOPER
diff --git a/src/core/SkPath.cpp b/src/core/SkPath.cpp
index c480624a16..60cfe0373c 100644
--- a/src/core/SkPath.cpp
+++ b/src/core/SkPath.cpp
@@ -2066,7 +2066,7 @@ SkPath::Verb SkPath::RawIter::next(SkPoint pts[4]) {
     Format in compressed buffer: [ptCount, verbCount, pts[], verbs[]]
 */
 
-size_t SkPath::writeToMemory(void* storage) const {
+uint32_t SkPath::writeToMemory(void* storage) const {
     SkDEBUGCODE(this->validate();)
 
     if (NULL == storage) {
@@ -2090,11 +2090,11 @@ size_t SkPath::writeToMemory(void* storage) const {
     fPathRef->writeToBuffer(&buffer);
 
     buffer.padToAlign4();
-    return buffer.pos();
+    return SkToU32(buffer.pos());
 }
 
-size_t SkPath::readFromMemory(const void* storage, size_t length) {
-    SkRBufferWithSizeCheck buffer(storage, length);
+uint32_t SkPath::readFromMemory(const void* storage) {
+    SkRBuffer   buffer(storage);
 
     uint32_t packed = buffer.readS32();
     fIsOval = (packed >> kIsOval_SerializationShift) & 1;
@@ -2108,18 +2108,14 @@ size_t SkPath::readFromMemory(const void* storage, size_t length) {
 
     fPathRef.reset(SkPathRef::CreateFromBuffer(&buffer
 #ifndef DELETE_THIS_CODE_WHEN_SKPS_ARE_REBUILT_AT_V14_AND_ALL_OTHER_INSTANCES_TOO
-        , newFormat, packed
+        , newFormat, packed)
 #endif
-        ));
+        );
 
     buffer.skipToAlign4();
 
-    size_t sizeRead = 0;
-    if (buffer.isValid()) {
-        SkDEBUGCODE(this->validate();)
-        sizeRead = buffer.pos();
-    }
-    return sizeRead;
+    SkDEBUGCODE(this->validate();)
+    return SkToU32(buffer.pos());
 }
 
 ///////////////////////////////////////////////////////////////////////////////
diff --git a/src/core/SkPicturePlayback.cpp b/src/core/SkPicturePlayback.cpp
index 5a016d48d6..f2d959d3d6 100644
--- a/src/core/SkPicturePlayback.cpp
+++ b/src/core/SkPicturePlayback.cpp
@@ -997,8 +997,7 @@ void SkPicturePlayback::draw(SkCanvas& canvas, SkDrawPictureCallback* callback)
             case DRAW_RRECT: {
                 const SkPaint& paint = *getPaint(reader);
                 SkRRect rrect;
-                reader.readRRect(&rrect);
-                canvas.drawRRect(rrect, paint);
+                canvas.drawRRect(*reader.readRRect(&rrect), paint);
             } break;
             case DRAW_SPRITE: {
                 const SkPaint* paint = getPaint(reader);
diff --git a/src/core/SkRRect.cpp b/src/core/SkRRect.cpp
index bcbf37ec59..e3d11cb01e 100644
--- a/src/core/SkRRect.cpp
+++ b/src/core/SkRRect.cpp
@@ -259,7 +259,7 @@ void SkRRect::inset(SkScalar dx, SkScalar dy, SkRRect* dst) const {
 
 ///////////////////////////////////////////////////////////////////////////////
 
-size_t SkRRect::writeToMemory(void* buffer) const {
+uint32_t SkRRect::writeToMemory(void* buffer) const {
     SkASSERT(kSizeInMemory == sizeof(SkRect) + sizeof(fRadii));
 
     memcpy(buffer, &fRect, sizeof(SkRect));
@@ -267,11 +267,7 @@ size_t SkRRect::writeToMemory(void* buffer) const {
     return kSizeInMemory;
 }
 
-size_t SkRRect::readFromMemory(const void* buffer, size_t length) {
-    if (length < kSizeInMemory) {
-        return 0;
-    }
-
+uint32_t SkRRect::readFromMemory(const void* buffer) {
     SkScalar storage[12];
     SkASSERT(sizeof(storage) == kSizeInMemory);
 
diff --git a/src/core/SkRegion.cpp b/src/core/SkRegion.cpp
index 468be67154..02994bffb0 100644
--- a/src/core/SkRegion.cpp
+++ b/src/core/SkRegion.cpp
@@ -1100,9 +1100,9 @@ bool SkRegion::op(const SkRegion& rgna, const SkRegion& rgnb, Op op) {
 
 #include "SkBuffer.h"
 
-size_t SkRegion::writeToMemory(void* storage) const {
+uint32_t SkRegion::writeToMemory(void* storage) const {
     if (NULL == storage) {
-        size_t size = sizeof(int32_t); // -1 (empty), 0 (rect), runCount
+        uint32_t size = sizeof(int32_t); // -1 (empty), 0 (rect), runCount
         if (!this->isEmpty()) {
             size += sizeof(fBounds);
             if (this->isComplex()) {
@@ -1133,11 +1133,11 @@ size_t SkRegion::writeToMemory(void* storage) const {
     return buffer.pos();
 }
 
-size_t SkRegion::readFromMemory(const void* storage, size_t length) {
-    SkRBufferWithSizeCheck  buffer(storage, length);
-    SkRegion                tmp;
-    int32_t                 count;
-    
+uint32_t SkRegion::readFromMemory(const void* storage) {
+    SkRBuffer   buffer(storage);
+    SkRegion    tmp;
+    int32_t     count;
+
     count = buffer.readS32();
     if (count >= 0) {
         buffer.read(&tmp.fBounds, sizeof(tmp.fBounds));
@@ -1150,12 +1150,8 @@ size_t SkRegion::readFromMemory(const void* storage, size_t length) {
             buffer.read(tmp.fRunHead->writable_runs(), count * sizeof(RunType));
         }
     }
-    size_t sizeRead = 0;
-    if (buffer.isValid()) {
-        this->swap(tmp);
-        sizeRead = buffer.pos();
-    }
-    return sizeRead;
+    this->swap(tmp);
+    return buffer.pos();
 }
 
 ///////////////////////////////////////////////////////////////////////////////
diff --git a/src/core/SkValidatingReadBuffer.cpp b/src/core/SkValidatingReadBuffer.cpp
index 3084565ffd..9f094f9617 100644
--- a/src/core/SkValidatingReadBuffer.cpp
+++ b/src/core/SkValidatingReadBuffer.cpp
@@ -118,11 +118,8 @@ void SkValidatingReadBuffer::readPoint(SkPoint* point) {
 }
 
 void SkValidatingReadBuffer::readMatrix(SkMatrix* matrix) {
-    size_t size = 0;
-    if (!fError) {
-        size = matrix->readFromMemory(fReader.peek(), fReader.available());
-        this->validate((SkAlign4(size) != size) || (0 == size));
-    }
+    const size_t size = matrix->readFromMemory(fReader.peek());
+    this->validate(SkAlign4(size) == size);
     if (!fError) {
         (void)this->skip(size);
     }
@@ -143,22 +140,16 @@ void SkValidatingReadBuffer::readRect(SkRect* rect) {
 }
 
 void SkValidatingReadBuffer::readRegion(SkRegion* region) {
-    size_t size = 0;
-    if (!fError) {
-        size = region->readFromMemory(fReader.peek(), fReader.available());
-        this->validate((SkAlign4(size) != size) || (0 == size));
-    }
+    const size_t size = region->readFromMemory(fReader.peek());
+    this->validate(SkAlign4(size) == size);
     if (!fError) {
         (void)this->skip(size);
     }
 }
 
 void SkValidatingReadBuffer::readPath(SkPath* path) {
-    size_t size = 0;
-    if (!fError) {
-        size = path->readFromMemory(fReader.peek(), fReader.available());
-        this->validate((SkAlign4(size) != size) || (0 == size));
-    }
+    const size_t size = path->readFromMemory(fReader.peek());
+    this->validate(SkAlign4(size) == size);
     if (!fError) {
         (void)this->skip(size);
     }
@@ -198,8 +189,6 @@ bool SkValidatingReadBuffer::readScalarArray(SkScalar* values, size_t size) {
 }
 
 uint32_t SkValidatingReadBuffer::getArrayCount() {
-    const size_t inc = sizeof(uint32_t);
-    fError = fError || !IsPtrAlign4(fReader.peek()) || !fReader.isAvailable(inc);
     return *(uint32_t*)fReader.peek();
 }