C++ bindings: A struct's Deserialize() now does validation before deserializing.

mojo/public/tools/bindings/generators/cpp_templates/struct_serialization_definition.tmpl

Index: mojo/public/tools/bindings/generators/cpp_templates/struct_serialization_definition.tmpl
diff --git a/mojo/public/tools/bindings/generators/cpp_templates/struct_serialization_definition.tmpl b/mojo/public/tools/bindings/generators/cpp_templates/struct_serialization_definition.tmpl
index 6ac6a3ea7ab800b3cc6c36cc7b4f17847e995f78..867ed6c8995b2a8f8bd971b9892100e19fd6f328 100644
--- a/mojo/public/tools/bindings/generators/cpp_templates/struct_serialization_definition.tmpl
+++ b/mojo/public/tools/bindings/generators/cpp_templates/struct_serialization_definition.tmpl
@@ -5,36 +5,66 @@ size_t {{struct.name}}::GetSerializedSize() const {
 }
 
 bool {{struct.name}}::Serialize(void* buf,
-                                size_t buf_size) {
+                                size_t buf_size,
+                                size_t* bytes_written) {
+  MOJO_DCHECK(buf);
+
   mojo::internal::FixedBuffer overlay_buf;
   overlay_buf.Initialize(buf, buf_size);
 
   internal::{{struct.name}}_Data* output_ptr;
-  if (Serialize_(this, &overlay_buf, &output_ptr) !=
-      mojo::internal::ValidationError::NONE) {
+  auto err = Serialize_(this, &overlay_buf, &output_ptr);
+  if (err != mojo::internal::ValidationError::NONE) {
     // TODO(vardhan): Once Serialize_() outputs handles that it serialized
     // (even partially, if there are failures), we should CHECK fail here if
     // handles are non-empty.
+    MOJO_DLOG(ERROR) << "Could not serialize: " <<
+        mojo::internal::ValidationErrorToString(err);
+
+    if (bytes_written)
+      *bytes_written = overlay_buf.BytesUsed();
     return false;
   }
 
   std::vector<mojo::Handle> handles;
   output_ptr->EncodePointersAndHandles(&handles);
-
   MOJO_CHECK(handles.empty()) << "Serialize() does not support handles.";
 
+  if (bytes_written)
+    *bytes_written = overlay_buf.BytesUsed();
   return true;
 }
 
-void {{struct.name}}::Deserialize(void* buf) {
+bool {{struct.name}}::Deserialize(void* buf, size_t buf_size) {

[viettrungluu, 2016/03/23 17:43:53]

const void*?

Also, can't/shouldn't Deserialize() (and DeserializeWithoutValidation()) tell
you how many bytes it actually "consumed"?

[vardhan, 2016/03/23 23:28:43]

discussed offline -- "bytes consumed" is hard to compute with pointers.

i think its possible if we enforce depth-first order when serializing a struct's
fields into a buffer: this way when deserializing, we could maintain the most
recent pointer (+ size of the object) we jump to since that would be the last
thing in the buffer.

either way, it would still require changes to the internal Deserialize_() which
involves changing some consumers.  so if this proves necessary, we could do it
in a later change.

+  MOJO_DCHECK(buf);
+
+  mojo::internal::BoundsChecker checker(buf, buf_size, 0);
+  std::string err_str;
+  mojo::internal::ValidationError err =
+      internal::{{struct.name}}_Data::Validate(buf, &checker, &err_str);
+  if (err != mojo::internal::ValidationError::NONE) {
+    MOJO_DLOG(ERROR) << "Deserialization error "
+                     << mojo::internal::ValidationErrorToString(err)
+                     << ": " << err_str;
+    return false;
+  }
+
+  return DeserializeWithoutValidation(buf, buf_size);
+}
+
+
+bool {{struct.name}}::DeserializeWithoutValidation(void* buf,

[viettrungluu, 2016/03/23 17:43:53]

const void*?

[vardhan, 2016/03/23 23:28:43]

Can't right now because Deserialization is currently mutable -- we rewrite
offsets to pointers, and resetting the encoded handle offsets.

I'll add a TODO here, but making it const involves getting rid of
DecodePointersAndHandles, which is on my list of things to do for the new C++
bindings generator.

+                                                   size_t buf_size) {
+  MOJO_DCHECK(buf);
+
   internal::{{struct.name}}_Data* input =
       static_cast<internal::{{struct.name}}_Data*>(buf);
-
   std::vector<mojo::Handle> handles;
   input->DecodePointersAndHandles(&handles);
   MOJO_CHECK(handles.empty()) << "Deserialize() does not support handles.";
 
   Deserialize_(input, this);
+  return true;

[viettrungluu, 2016/03/23 17:43:53]

Then why does it return anything at all?

[vardhan, 2016/03/23 23:28:43]

Done.

 }
 
 size_t GetSerializedSize_(const {{struct.name}}& input) {