Add buffer helper to read directly into SkString to avoid extra copying

Add buffer helper to read directly into SkString to avoid extra copying

R=reed@google.com

Committed: https://code.google.com/p/skia/source/detail?r=9218

[reed1, 2013-05-20 13:42:47]

https://codereview.chromium.org/15179008/diff/1/include/core/SkFlattenableBuf...
File include/core/SkFlattenableBuffers.h (right):

https://codereview.chromium.org/15179008/diff/1/include/core/SkFlattenableBuf...
include/core/SkFlattenableBuffers.h:63: virtual void readString(SkString&) = 0;
Use * instead of & for mutable arguments

https://codereview.chromium.org/15179008/diff/1/src/core/SkOrderedReadBuffer.cpp
File src/core/SkOrderedReadBuffer.cpp (right):

https://codereview.chromium.org/15179008/diff/1/src/core/SkOrderedReadBuffer....
src/core/SkOrderedReadBuffer.cpp:93: string.set(fReader.readString(&len), len);
Parameter evaluation order is undefined, so it is possible that the 2nd arg will
be evaluated first, passing an uninitialized value for len. Probably should use
the same pattern as we do below in readString().

https://codereview.chromium.org/15179008/diff/1/src/core/SkOrderedReadBuffer....
src/core/SkOrderedReadBuffer.cpp:96: char* SkOrderedReadBuffer::readString() {
Suggestion: rename this to something other than String, indicating that it was
allocated via malloc.

e.g.
readCString
readCStringWithMalloc

... do we even need it, how that you're adding a cool version of readString?

[djsollen, 2013-05-21 12:17:17]

https://codereview.chromium.org/15179008/diff/1/include/core/SkFlattenableBuf...
File include/core/SkFlattenableBuffers.h (right):

https://codereview.chromium.org/15179008/diff/1/include/core/SkFlattenableBuf...
include/core/SkFlattenableBuffers.h:63: virtual void readString(SkString&) = 0;
On 2013/05/20 13:42:47, reed1 wrote:
> Use * instead of & for mutable arguments

Done.

https://codereview.chromium.org/15179008/diff/1/src/core/SkOrderedReadBuffer.cpp
File src/core/SkOrderedReadBuffer.cpp (right):

https://codereview.chromium.org/15179008/diff/1/src/core/SkOrderedReadBuffer....
src/core/SkOrderedReadBuffer.cpp:93: string.set(fReader.readString(&len), len);
On 2013/05/20 13:42:47, reed1 wrote:
> Parameter evaluation order is undefined, so it is possible that the 2nd arg
will
> be evaluated first, passing an uninitialized value for len. Probably should
use
> the same pattern as we do below in readString().

Done.

https://codereview.chromium.org/15179008/diff/1/src/core/SkOrderedReadBuffer....
src/core/SkOrderedReadBuffer.cpp:96: char* SkOrderedReadBuffer::readString() {
removed and updated the one caller to use the new readString call.

On 2013/05/20 13:42:47, reed1 wrote:
> Suggestion: rename this to something other than String, indicating that it was
> allocated via malloc.
> 
> e.g.
> readCString
> readCStringWithMalloc
> 
> ... do we even need it, how that you're adding a cool version of readString?

[reed1, 2013-05-21 15:45:09]

lgtm

[djsollen, 2013-05-21 16:17:21]

Committed patchset #3 manually as r9218 (presubmit successful).