Adding size parameter to read array functions

include/core/SkFlattenableBuffers.h

Index: include/core/SkFlattenableBuffers.h
diff --git a/include/core/SkFlattenableBuffers.h b/include/core/SkFlattenableBuffers.h
index 6e44550be561c6f267ade5f6655557a03d2edb3f..ce19b12bbdac7a4627bac1c71a369c51a46e9d17 100644
--- a/include/core/SkFlattenableBuffers.h
+++ b/include/core/SkFlattenableBuffers.h
@@ -99,11 +99,20 @@ public:
     virtual void readPath(SkPath* path) = 0;
 
     // binary data and arrays
-    virtual uint32_t readByteArray(void* value) = 0;
-    virtual uint32_t readColorArray(SkColor* colors) = 0;
-    virtual uint32_t readIntArray(int32_t* values) = 0;
-    virtual uint32_t readPointArray(SkPoint* points) = 0;
-    virtual uint32_t readScalarArray(SkScalar* values) = 0;
+
+    /**
+      * In the following read.*Array(...) functions, the size parameter specifies the allocation
+      * size (in bytes) of the pointer parameter. In the case where isValidating() is true, then
+      * if the pointer parameter's size does not match the size to be read, the pointer
+      * parameter's memory will then stay uninitialized, the function will return 0 and an error
+      * flag will be set internally (see SkValidatingReadBuffer).
+      * If isValidating() is false, then the size parameter is ignored.

[reed1, 2013/10/29 19:39:43]

What gets return if there is *not* an error? (in both validating and
non-validating buffers)

Seems like non-validating could perform checks:
- only memcpy the min of the embedded size and the provided size

[sugoi1, 2013/10/29 20:04:50]

It returns the size (or amount) of memory actually read. I'll update the
description.
Will add. I just wanted to minimize changes to non-validating code, but adding
this should be trivial.

+      */
+    virtual uint32_t readByteArray(void* value, uint32_t size) = 0;

[reed1, 2013/10/29 19:39:43]

shouldn't these be size_t instead of uint32_t, just for clarity/consistency
sake? Same comment for the return value.

[reed1, 2013/10/29 19:44:09]

What is the state of the buffer if we get an error? How much has the cursor been
advanced?

[sugoi1, 2013/10/29 20:04:50]

Will add in the description. Thanks.

[sugoi1, 2013/10/29 20:04:50]

Sure, but I'll just mention that, currently, the underlying reader is
SkReader32, so using uint32_t prevents overflowing the reader's memory with a
potentially 64b size_t, on 64b platforms (For example, SkReader32::size(),
SkReader32::offset() and SkReader32::available() all return a uint32_t). Now, I
understand that this interface should be reader agnostic, so I'll change it.
Maybe the reader could support 64b on 64b platforms eventually ?

+    virtual uint32_t readColorArray(SkColor* colors, uint32_t size) = 0;
+    virtual uint32_t readIntArray(int32_t* values, uint32_t size) = 0;
+    virtual uint32_t readPointArray(SkPoint* points, uint32_t size) = 0;
+    virtual uint32_t readScalarArray(SkScalar* values, uint32_t size) = 0;
 
     /** This helper peeks into the buffer and reports back the length of the next array in
      *  the buffer but does not change the state of the buffer.
@@ -127,7 +136,7 @@ public:
     SkData* readByteArrayAsData() {
         size_t len = this->getArrayCount();
         void* buffer = sk_malloc_throw(len);
-        (void)this->readByteArray(buffer);
+        (void)this->readByteArray(buffer, len);
         return SkData::NewFromMalloc(buffer, len);
     }