Follow up to serialization validation code

src/core/SkValidatingReadBuffer.cpp

 /*
  * Copyright 2013 Google Inc.
  *
  * Use of this source code is governed by a BSD-style license that can be
  * found in the LICENSE file.
  */
 
 #include "SkBitmap.h"
 #include "SkErrorInternals.h"
 #include "SkValidatingReadBuffer.h"
@@ skipping 26 matching lines @@
     }
     return addr;
 }
 
 // All the methods in this file funnel down into either readInt(), readScalar() or skip(),
 // followed by a memcpy. So we've got all our validation in readInt(), readScalar() and skip();
 // if they fail they'll return a zero value or skip nothing, respectively, and set fError to
 // true, which the caller should check to see if an error occurred during the read operation.
 
 bool SkValidatingReadBuffer::readBool() {
-    return this->readInt() != 0;
+    uint32_t value = this->readInt();

[Stephen White, 2013/10/23 18:09:35]

Not new to this patch, but some flavours of writeBool() call write8(),
SkWriter::writeBool() calls writeInt() (without explicitly converting to 0, 1).
The readers do a readInt(). Kinda weird.

[sugoi1, 2013/10/23 18:17:08]

Yeah, I had to check that noone was doing any kind of funny business with the
bools to make sure skia never writes anything other than 0 or 1 when writing a
bool.

+    // Boolean value should be either 0 or 1
+    if (value & 0xFFFFFFFE) {

[reed1, 2013/10/23 17:58:42]

this is correct, but possibly hard for a reviewer to confirm. What about?

if (value & ~1) ...

[sugoi1, 2013/10/23 18:07:43]

Done.

+        fError = true;
+    }
+    return value != 0;
 }
 
 SkColor SkValidatingReadBuffer::readColor() {
     return this->readInt();
 }
 
 SkFixed SkValidatingReadBuffer::readFixed() {
     return this->readInt();
 }
 
@@ skipping 192 matching lines @@
             delete obj;
             obj = NULL;
         }
     } else {
         // we must skip the remaining data
         this->skip(sizeRecorded);
         SkASSERT(false);
     }
     return obj;
 }