Avoid undefined behavior when checking for pointer wraparound

base/pickle.cc

Index: base/pickle.cc
===================================================================
--- base/pickle.cc	(revision 178847)
+++ base/pickle.cc	(working copy)
@@ -35,12 +35,14 @@
 template<typename Type>
 inline const char* PickleIterator::GetReadPointerAndAdvance() {
   const char* current_read_ptr = read_ptr_;
-  if (read_ptr_ + sizeof(Type) > read_end_ptr_)
-    return NULL;
+  size_t s;

[jar (doing other things), 2013/03/04 19:25:51]

nit: style guide suggests avoiding single character names.

Suggest:

size_t type_size;

   if (sizeof(Type) < sizeof(uint32))
-    read_ptr_ += AlignInt(sizeof(Type), sizeof(uint32));
+    s = AlignInt(sizeof(Type), sizeof(uint32));

[jar (doing other things), 2013/03/04 19:25:51]

Am I correct in reading AlignInt(smaller, larger) as being equal to larger?

So this is just:
s = sizeof(uint32);

IF so, we may as well write line 38:
size_t = std::max(sizeof(Type), sizeof(uint32));

and toss lines 39-42.  It would be much clearer.

Am I missing anything?

   else
-    read_ptr_ += sizeof(Type);
+    s = sizeof(Type);
+  if (s > static_cast<size_t>(read_end_ptr_ - read_ptr_))
+    return NULL;
+  read_ptr_ += s;
   return current_read_ptr;
 }
 
@@ -353,9 +355,8 @@
 
   const Header* hdr = reinterpret_cast<const Header*>(start);
   const char* payload_base = start + header_size;
-  const char* payload_end = payload_base + hdr->payload_size;
-  if (payload_end < payload_base)
+  if (hdr->payload_size > static_cast<size_t>(end - payload_base))
     return NULL;
 
-  return (payload_end > end) ? NULL : payload_end;
+  return payload_base + hdr->payload_size;
 }