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 74 matching lines @@
     size_t inc = SkAlign4(size);
     const void* addr = fReader.peek();
     fError |= !ptr_align_4(addr) || !fReader.isAvailable(inc);
     if (!fError) {
         fReader.skip(size);
     }
     return addr;
 }
 
 bool SkValidatingReadBuffer::readBool() {
+    const void* addr = fReader.peek();
+    const uint32_t* boolPtr = static_cast<const uint32_t*>(addr);
+    // Boolean value should be either 0 or 1
+    fError |= !boolPtr || (*boolPtr) & 0xFFFFFFFE;

[reed1, 2013/09/13 15:42:56]

Why are we dealing with ptrs and casts here? Why not...

uint32_t value = this->readInt();
if (0 != value || 1 != value) {
    fError = true;
}
return value;

[sugoi1, 2013/09/13 16:53:44]

You're right, I was originally thinking of a more complex case where I wouldn't
want to read an entire, larger object, but here, it's already reading the entire
int value to perform the check so this is redundant.

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