Adding error checks to SkRBuffer

src/core/SkPath.cpp

Index: src/core/SkPath.cpp
diff --git a/src/core/SkPath.cpp b/src/core/SkPath.cpp
index c480624a166a287a56701259adf8609c4bdf2dee..2fc8947ec0e7d86f3ee0815fe6249face717e87b 100644
--- a/src/core/SkPath.cpp
+++ b/src/core/SkPath.cpp
@@ -2082,8 +2082,9 @@ size_t SkPath::writeToMemory(void* storage) const {
                      (fSegmentMask << kSegmentMask_SerializationShift) |
                      (fDirection << kDirection_SerializationShift)
 #ifndef DELETE_THIS_CODE_WHEN_SKPS_ARE_REBUILT_AT_V14_AND_ALL_OTHER_INSTANCES_TOO
-                     | (0x1 << kNewFormat_SerializationShift);
+                     | (0x1 << kNewFormat_SerializationShift)
 #endif
+                     ;
 
     buffer.write32(packed);
 
@@ -2096,7 +2097,11 @@ size_t SkPath::writeToMemory(void* storage) const {
 size_t SkPath::readFromMemory(const void* storage, size_t length) {
     SkRBufferWithSizeCheck buffer(storage, length);
 
-    uint32_t packed = buffer.readS32();
+    int32_t packed;
+    if (!buffer.readS32(&packed)) {
+        return 0;
+    }
+
     fIsOval = (packed >> kIsOval_SerializationShift) & 1;
     fConvexity = (packed >> kConvexity_SerializationShift) & 0xFF;
     fFillType = (packed >> kFillType_SerializationShift) & 0xFF;
@@ -2106,18 +2111,22 @@ size_t SkPath::readFromMemory(const void* storage, size_t length) {
     bool newFormat = (packed >> kNewFormat_SerializationShift) & 1;
 #endif
 
-    fPathRef.reset(SkPathRef::CreateFromBuffer(&buffer
+    SkPathRef* pathRef = SkPathRef::CreateFromBuffer(&buffer
 #ifndef DELETE_THIS_CODE_WHEN_SKPS_ARE_REBUILT_AT_V14_AND_ALL_OTHER_INSTANCES_TOO
         , newFormat, packed
 #endif
-        ));
-
-    buffer.skipToAlign4();
+        );
 
     size_t sizeRead = 0;
     if (buffer.isValid()) {
+        fPathRef.reset(pathRef);
         SkDEBUGCODE(this->validate();)
+        buffer.skipToAlign4();
         sizeRead = buffer.pos();
+    } else if (NULL != pathRef) {
+        // If the buffer is not valid, pathRef should be NULL

[sugoi1, 2013/11/08 16:20:06]

Although pathRef should always be NULL if the buffer isn't valid, I added a
safety check here to make sure that it is the case.

[reed1, 2013/11/08 18:15:39]

should we call sk_throw()? Why break in the debug build, but not in release? Is
this really an error, or just mysterious?

[sugoi1, 2013/11/08 18:40:56]

Done. Changed to sk_throw(). This should really never happen.

+        SkASSERT(false);
+        SkDELETE(pathRef);
     }
     return sizeRead;
 }