Mojo cpp bindings: remove redundant validation in Decode*().

Because we already do necessary checks during the validation stage.

BUG=None
TEST=None

Review URL: https://codereview.chromium.org/293983026

git-svn-id: svn://svn.chromium.org/chrome/trunk/src@274089 0039d316-1c4b-4281-b951-d872f2087c98

9 files changed, 39 insertions, 72 deletions

diff --git a/mojo/public/cpp/bindings/lib/array_internal.cc b/mojo/public/cpp/bindings/lib/array_internal.cc
index 4614bd1..18443ed 100644
--- a/mojo/public/cpp/bindings/lib/array_internal.cc
+++ b/mojo/public/cpp/bindings/lib/array_internal.cc
@@ -44,15 +44,12 @@ void ArraySerializationHelper<Handle, true>::EncodePointersAndHandles(
 }
 
 // static
-bool ArraySerializationHelper<Handle, true>::DecodePointersAndHandles(
+void ArraySerializationHelper<Handle, true>::DecodePointersAndHandles(
     const ArrayHeader* header,
     ElementType* elements,
-    Message* message) {
-  for (uint32_t i = 0; i < header->num_elements; ++i) {
-    if (!DecodeHandle(&elements[i], message->mutable_handles()))
-      return false;
-  }
-  return true;
+    std::vector<Handle>* handles) {
+  for (uint32_t i = 0; i < header->num_elements; ++i)
+    DecodeHandle(&elements[i], handles);
 }
 
 // static
diff --git a/mojo/public/cpp/bindings/lib/array_internal.h b/mojo/public/cpp/bindings/lib/array_internal.h
index c1cf3e4..28a0a0d 100644
--- a/mojo/public/cpp/bindings/lib/array_internal.h
+++ b/mojo/public/cpp/bindings/lib/array_internal.h
@@ -106,10 +106,9 @@ struct ArraySerializationHelper<T, false> {
                                        std::vector<Handle>* handles) {
   }
 
-  static bool DecodePointersAndHandles(const ArrayHeader* header,
+  static void DecodePointersAndHandles(const ArrayHeader* header,
                                        ElementType* elements,
-                                       Message* message) {
-    return true;
+                                       std::vector<Handle>* handles) {
   }
 
   static bool ValidateElements(const ArrayHeader* header,
@@ -127,9 +126,9 @@ struct ArraySerializationHelper<Handle, true> {
                                        ElementType* elements,
                                        std::vector<Handle>* handles);
 
-  static bool DecodePointersAndHandles(const ArrayHeader* header,
+  static void DecodePointersAndHandles(const ArrayHeader* header,
                                        ElementType* elements,
-                                       Message* message);
+                                       std::vector<Handle>* handles);
 
   static bool ValidateElements(const ArrayHeader* header,
                                const ElementType* elements,
@@ -147,11 +146,11 @@ struct ArraySerializationHelper<H, true> {
         header, elements, handles);
   }
 
-  static bool DecodePointersAndHandles(const ArrayHeader* header,
+  static void DecodePointersAndHandles(const ArrayHeader* header,
                                        ElementType* elements,
-                                       Message* message) {
-    return ArraySerializationHelper<Handle, true>::DecodePointersAndHandles(
-        header, elements, message);
+                                       std::vector<Handle>* handles) {
+    ArraySerializationHelper<Handle, true>::DecodePointersAndHandles(
+        header, elements, handles);
   }
 
   static bool ValidateElements(const ArrayHeader* header,
@@ -173,14 +172,11 @@ struct ArraySerializationHelper<P*, false> {
       Encode(&elements[i], handles);
   }
 
-  static bool DecodePointersAndHandles(const ArrayHeader* header,
+  static void DecodePointersAndHandles(const ArrayHeader* header,
                                        ElementType* elements,
-                                       Message* message) {
-    for (uint32_t i = 0; i < header->num_elements; ++i) {
-      if (!Decode(&elements[i], message))
-        return false;
-    }
-    return true;
+                                       std::vector<Handle>* handles) {
+    for (uint32_t i = 0; i < header->num_elements; ++i)
+      Decode(&elements[i], handles);
   }
 
   static bool ValidateElements(const ArrayHeader* header,
@@ -258,8 +254,8 @@ class Array_Data {
     Helper::EncodePointersAndHandles(&header_, storage(), handles);
   }
 
-  bool DecodePointersAndHandles(Message* message) {
-    return Helper::DecodePointersAndHandles(&header_, storage(), message);
+  void DecodePointersAndHandles(std::vector<Handle>* handles) {
+    Helper::DecodePointersAndHandles(&header_, storage(), handles);
   }
 
  private:
diff --git a/mojo/public/cpp/bindings/lib/bindings_serialization.cc b/mojo/public/cpp/bindings/lib/bindings_serialization.cc
index 6f5fa3d..c1114c0 100644
--- a/mojo/public/cpp/bindings/lib/bindings_serialization.cc
+++ b/mojo/public/cpp/bindings/lib/bindings_serialization.cc
@@ -60,17 +60,6 @@ bool ValidateEncodedPointer(const uint64_t* offset) {
       reinterpret_cast<uintptr_t>(offset);
 }
 
-bool ValidatePointer(const void* ptr, const Message& message) {
-  const uint8_t* data = static_cast<const uint8_t*>(ptr);
-  if (reinterpret_cast<uintptr_t>(data) % 8 != 0)
-    return false;
-
-  const uint8_t* data_start = message.data();
-  const uint8_t* data_end = data_start + message.data_num_bytes();
-
-  return data >= data_start && data < data_end;
-}
-
 void EncodeHandle(Handle* handle, std::vector<Handle>* handles) {
   if (handle->is_valid()) {
     handles->push_back(*handle);
@@ -80,16 +69,14 @@ void EncodeHandle(Handle* handle, std::vector<Handle>* handles) {
   }
 }
 
-bool DecodeHandle(Handle* handle, std::vector<Handle>* handles) {
+void DecodeHandle(Handle* handle, std::vector<Handle>* handles) {
   if (handle->value() == kEncodedInvalidHandleValue) {
     *handle = Handle();
-    return true;
+    return;
   }
-  if (handle->value() >= handles->size())
-    return false;
+  assert(handle->value() < handles->size());
   // Just leave holes in the vector so we don't screw up other indices.
   *handle = FetchAndReset(&handles->at(handle->value()));
-  return true;
 }
 
 bool ValidateStructHeader(const void* data,
diff --git a/mojo/public/cpp/bindings/lib/bindings_serialization.h b/mojo/public/cpp/bindings/lib/bindings_serialization.h
index acb5575..9cd2f13 100644
--- a/mojo/public/cpp/bindings/lib/bindings_serialization.h
+++ b/mojo/public/cpp/bindings/lib/bindings_serialization.h
@@ -7,7 +7,7 @@
 
 #include <vector>
 
-#include "mojo/public/cpp/bindings/message.h"
+#include "mojo/public/cpp/system/core.h"
 
 namespace mojo {
 namespace internal {
@@ -32,8 +32,10 @@ bool IsAligned(const void* ptr);
 // A null pointer is encoded as an offset value of 0.
 //
 void EncodePointer(const void* ptr, uint64_t* offset);
+// Note: This function doesn't validate the encoded pointer value.
 const void* DecodePointerRaw(const uint64_t* offset);
 
+// Note: This function doesn't validate the encoded pointer value.
 template <typename T>
 inline void DecodePointer(const uint64_t* offset, T** ptr) {
   *ptr = reinterpret_cast<T*>(const_cast<void*>(DecodePointerRaw(offset)));
@@ -43,13 +45,11 @@ inline void DecodePointer(const uint64_t* offset, T** ptr) {
 // smaller than |offset|.
 bool ValidateEncodedPointer(const uint64_t* offset);
 
-// Check that the given pointer references memory contained within the message.
-bool ValidatePointer(const void* ptr, const Message& message);
-
 // Handles are encoded as indices into a vector of handles. These functions
 // manipulate the value of |handle|, mapping it to and from an index.
 void EncodeHandle(Handle* handle, std::vector<Handle>* handles);
-bool DecodeHandle(Handle* handle, std::vector<Handle>* handles);
+// Note: This function doesn't validate the encoded handle value.
+void DecodeHandle(Handle* handle, std::vector<Handle>* handles);
 
 // The following 2 functions are used to encode/decode all objects (structs and
 // arrays) in a consistent manner.
@@ -61,18 +61,12 @@ inline void Encode(T* obj, std::vector<Handle>* handles) {
   EncodePointer(obj->ptr, &obj->offset);
 }
 
-// TODO(yzshen): Remove all redundant validation during decoding. And make
-// Decode*() functions/methods return void.
+// Note: This function doesn't validate the encoded pointer and handle values.
 template <typename T>
-inline bool Decode(T* obj, Message* message) {
+inline void Decode(T* obj, std::vector<Handle>* handles) {
   DecodePointer(&obj->offset, &obj->ptr);
-  if (obj->ptr) {
-    if (!ValidatePointer(obj->ptr, *message))
-      return false;
-    if (!obj->ptr->DecodePointersAndHandles(message))
-      return false;
-  }
-  return true;
+  if (obj->ptr)
+    obj->ptr->DecodePointersAndHandles(handles);
 }
 
 // If returns true, this function also claims the memory range of the size
diff --git a/mojo/public/tools/bindings/generators/cpp_templates/interface_definition.tmpl b/mojo/public/tools/bindings/generators/cpp_templates/interface_definition.tmpl
index fe0a710..5df835c 100644
--- a/mojo/public/tools/bindings/generators/cpp_templates/interface_definition.tmpl
+++ b/mojo/public/tools/bindings/generators/cpp_templates/interface_definition.tmpl
@@ -100,8 +100,7 @@ bool {{class_name}}_{{method.name}}_ForwardToCallback::Accept(
       reinterpret_cast<internal::{{class_name}}_{{method.name}}_ResponseParams_Data*>(
           message->mutable_payload());
 
-  if (!params->DecodePointersAndHandles(message))
-    return false;
+  params->DecodePointersAndHandles(message->mutable_handles());
   {{alloc_params(method.response_parameters)|indent(2)}}
   callback_.Run({{pass_params(method.response_parameters)}});
   return true;
@@ -205,8 +204,7 @@ bool {{class_name}}Stub::Accept(mojo::Message* message) {
           reinterpret_cast<internal::{{class_name}}_{{method.name}}_Params_Data*>(
               message->mutable_payload());
 
-      if (!params->DecodePointersAndHandles(message))
-        return false;
+      params->DecodePointersAndHandles(message->mutable_handles());
       {{alloc_params(method.parameters)|indent(6)}}
       sink_->{{method.name}}({{pass_params(method.parameters)}});
       return true;
@@ -231,8 +229,7 @@ bool {{class_name}}Stub::AcceptWithResponder(
           reinterpret_cast<internal::{{class_name}}_{{method.name}}_Params_Data*>(
               message->mutable_payload());
 
-      if (!params->DecodePointersAndHandles(message))
-        return false;
+      params->DecodePointersAndHandles(message->mutable_handles());
       {{interface_macros.declare_callback(method)}}::Runnable* runnable =
           new {{class_name}}_{{method.name}}_ProxyToResponder(
               message->request_id(), responder);
diff --git a/mojo/public/tools/bindings/generators/cpp_templates/params_definition.tmpl b/mojo/public/tools/bindings/generators/cpp_templates/params_definition.tmpl
index 2625cfa..0b11047 100644
--- a/mojo/public/tools/bindings/generators/cpp_templates/params_definition.tmpl
+++ b/mojo/public/tools/bindings/generators/cpp_templates/params_definition.tmpl
@@ -19,9 +19,8 @@ class {{class_name}} {
     {{ struct_macros.encodes(struct)|indent(4) }}
   }
 
-  bool DecodePointersAndHandles(mojo::Message* message) {
+  void DecodePointersAndHandles(std::vector<mojo::Handle>* handles) {
     {{ struct_macros.decodes(struct)|indent(4) }}
-    return true;
   }
 
  private:
diff --git a/mojo/public/tools/bindings/generators/cpp_templates/struct_declaration.tmpl b/mojo/public/tools/bindings/generators/cpp_templates/struct_declaration.tmpl
index 0417d5f..60a6a9e 100644
--- a/mojo/public/tools/bindings/generators/cpp_templates/struct_declaration.tmpl
+++ b/mojo/public/tools/bindings/generators/cpp_templates/struct_declaration.tmpl
@@ -12,7 +12,7 @@ class {{class_name}} {
 {{struct_macros.fields(struct)}}
 
   void EncodePointersAndHandles(std::vector<mojo::Handle>* handles);
-  bool DecodePointersAndHandles(mojo::Message* message);
+  void DecodePointersAndHandles(std::vector<mojo::Handle>* handles);
 
  private:
   {{class_name}}();
diff --git a/mojo/public/tools/bindings/generators/cpp_templates/struct_definition.tmpl b/mojo/public/tools/bindings/generators/cpp_templates/struct_definition.tmpl
index b5263f9..461f158 100644
--- a/mojo/public/tools/bindings/generators/cpp_templates/struct_definition.tmpl
+++ b/mojo/public/tools/bindings/generators/cpp_templates/struct_definition.tmpl
@@ -22,7 +22,7 @@ void {{class_name}}::EncodePointersAndHandles(
   {{ struct_macros.encodes(struct)|indent(2) }}
 }
 
-bool {{class_name}}::DecodePointersAndHandles(mojo::Message* message) {
+void {{class_name}}::DecodePointersAndHandles(
+    std::vector<mojo::Handle>* handles) {
   {{ struct_macros.decodes(struct)|indent(2) }}
-  return true;
 }
diff --git a/mojo/public/tools/bindings/generators/cpp_templates/struct_macros.tmpl b/mojo/public/tools/bindings/generators/cpp_templates/struct_macros.tmpl
index 1897a49..4d586f7 100644
--- a/mojo/public/tools/bindings/generators/cpp_templates/struct_macros.tmpl
+++ b/mojo/public/tools/bindings/generators/cpp_templates/struct_macros.tmpl
@@ -78,12 +78,9 @@ mojo::internal::EncodeHandle(&{{pf.field.name}}, handles);
 
 {%- macro decodes(struct) -%}
 {%-   for pf in struct.packed.packed_fields if pf.field.kind|is_object_kind -%}
-if (!mojo::internal::Decode(&{{pf.field.name}}, message))
-  return false;
+mojo::internal::Decode(&{{pf.field.name}}, handles);
 {%    endfor %}
 {%-   for pf in struct.packed.packed_fields if pf.field.kind|is_handle_kind -%}
-if (!mojo::internal::DecodeHandle(&{{pf.field.name}},
-                                  message->mutable_handles()))
-  return false;
+mojo::internal::DecodeHandle(&{{pf.field.name}}, handles);
 {%    endfor %}
 {%- endmacro -%}