allow NULL in writeString/readString

allow NULL in writeString/readString

BUG=skia:1469
, code.google.com/p/android/issues/detail?id=58257

Committed: http://code.google.com/p/skia/source/detail?r=10662

[mtklein, 2013-08-09 15:07:13]

Seemed the easiest way to fix things is just to allow reading and writing NULL
instead of guarding against it everywhere.

[scroggo, 2013-08-09 15:18:28]

> Seemed the easiest way to fix things is just to allow
> reading and writing NULL instead of guarding against it
> everywhere.

Agreed.

lgtm

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

https://codereview.chromium.org/22359003/diff/1/src/core/SkWriter32.cpp#newco...
src/core/SkWriter32.cpp:249: if (len == 0xFFFF) {
What do you think about setting a constant for this?

[mtklein, 2013-08-09 15:22:49]

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

https://codereview.chromium.org/22359003/diff/1/src/core/SkWriter32.cpp#newco...
src/core/SkWriter32.cpp:249: if (len == 0xFFFF) {
On 2013/08/09 15:18:28, scroggo wrote:
> What do you think about setting a constant for this?

You know, I think it helps to see 0xFFFF itself as a number, at least in
writeString().  If you agree with me on that, it just seems clearest to have
0xFFFF here too for symmetry.

[scroggo, 2013-08-09 15:23:53]

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

https://codereview.chromium.org/22359003/diff/1/src/core/SkWriter32.cpp#newco...
src/core/SkWriter32.cpp:249: if (len == 0xFFFF) {
On 2013/08/09 15:22:49, mtklein wrote:
> On 2013/08/09 15:18:28, scroggo wrote:
> > What do you think about setting a constant for this?
> 
> You know, I think it helps to see 0xFFFF itself as a number, at least in
> writeString().  If you agree with me on that, it just seems clearest to have
> 0xFFFF here too for symmetry.

Okay with me.

[commit-bot: I haz the power, 2013-08-09 15:30:22]

CQ is trying da patch. Follow status at
https://skia-tree-status.appspot.com/cq/mtklein@google.com/22359003/8001

[commit-bot: I haz the power, 2013-08-09 16:03:08]

Change committed as 10662

[reed1, 2013-08-12 13:00:05]

What is the 0xFFFF for?

[mtklein, 2013-08-12 13:47:36]

On 2013/08/12 13:00:05, reed1 wrote:
> What is the 0xFFFF for?

SkWriter32::writeString() normally writes [ 4 byte length ] [ length + 1 bytes
of data and \0 ].  E.g. { 1, 97, 0 } encodes "a", { 0, 0 } encodes "", etc.  (We
like this because it lets us return a valid const char* pointing into the stream
without copying.)

writeString() requires len < 0xFFFF, so we could never normally write it as
those first 4 bytes.  This leaves { 0xFFFF } as a convenient way to encode NULL
and to allow NULL to be distinguished from "".

[reed1, 2013-08-12 14:31:22]

On 2013/08/12 13:47:36, mtklein wrote:
> On 2013/08/12 13:00:05, reed1 wrote:
> > What is the 0xFFFF for?
> 
> SkWriter32::writeString() normally writes [ 4 byte length ] [ length + 1 bytes
> of data and \0 ].  E.g. { 1, 97, 0 } encodes "a", { 0, 0 } encodes "", etc. 
(We
> like this because it lets us return a valid const char* pointing into the
stream
> without copying.)
> 
> writeString() requires len < 0xFFFF, so we could never normally write it as
> those first 4 bytes.  This leaves { 0xFFFF } as a convenient way to encode
NULL
> and to allow NULL to be distinguished from "".

Hmmm, but "why" does it require len < 0xFFFF, and do we actually need to encode
NULL strings? I don't see that mentioned in the dox...