Mojo: Refactor the verification of various |Mojo...Options| structs.

This changes the semantics slightly:
* All fields of |Mojo...Options|, except |struct_size| are optional.
  (The presence of fields is checked using |struct_size|.)
* |struct_size| isn't strictly checked; any |struct_size| that can at
  least contain the |struct_size| member is accepted.
* |Mojo...Options| with unknown flags are rejected with
  |MOJO_RESULT_UNIMPLEMENTED|. This will allow us to allow callers to
  specify that certain fields must not be ignored (even if running on a
  system version that doesn't recognize the field).

We add some utililities to make the required checks easier/more
consistent.

R=sky@chromium.org

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

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

8 files changed, 423 insertions, 62 deletions

diff --git a/mojo/system/data_pipe.cc b/mojo/system/data_pipe.cc
index dd465df..8dcc94c 100644
--- a/mojo/system/data_pipe.cc
+++ b/mojo/system/data_pipe.cc
@@ -13,6 +13,7 @@
 #include "base/logging.h"
 #include "mojo/system/constants.h"
 #include "mojo/system/memory.h"
+#include "mojo/system/options_validation.h"
 #include "mojo/system/waiter_list.h"
 
 namespace mojo {
@@ -28,36 +29,45 @@ MojoResult DataPipe::ValidateOptions(
     1u,
     static_cast<uint32_t>(kDefaultDataPipeCapacityBytes)
   };
-  if (!in_options) {
-    *out_options = kDefaultOptions;
+  *out_options = kDefaultOptions;
+
+  if (!in_options)
     return MOJO_RESULT_OK;
-  }
 
-  if (in_options->struct_size < sizeof(*in_options))
+  if (!IsOptionsStructPointerAndSizeValid<MojoCreateDataPipeOptions>(
+          in_options))
     return MOJO_RESULT_INVALID_ARGUMENT;
-  out_options->struct_size = static_cast<uint32_t>(sizeof(*out_options));
 
-  // All flags are okay (unrecognized flags will be ignored).
+  if (!HAS_OPTIONS_STRUCT_MEMBER(MojoCreateDataPipeOptions, flags, in_options))
+    return MOJO_RESULT_OK;
+  if (!AreOptionsFlagsAllKnown<MojoCreateDataPipeOptions>(
+          in_options, MOJO_CREATE_DATA_PIPE_OPTIONS_FLAG_MAY_DISCARD))
+    return MOJO_RESULT_UNIMPLEMENTED;
   out_options->flags = in_options->flags;
 
+  if (!HAS_OPTIONS_STRUCT_MEMBER(MojoCreateDataPipeOptions, element_num_bytes,
+                                 in_options))
+    return MOJO_RESULT_OK;
   if (in_options->element_num_bytes == 0)
     return MOJO_RESULT_INVALID_ARGUMENT;
   out_options->element_num_bytes = in_options->element_num_bytes;
 
-  if (in_options->capacity_num_bytes == 0) {
+  if (!HAS_OPTIONS_STRUCT_MEMBER(MojoCreateDataPipeOptions, capacity_num_bytes,
+                                 in_options) ||
+      in_options->capacity_num_bytes == 0) {
     // Round the default capacity down to a multiple of the element size (but at
     // least one element).
     out_options->capacity_num_bytes = std::max(
         static_cast<uint32_t>(kDefaultDataPipeCapacityBytes -
-            (kDefaultDataPipeCapacityBytes % in_options->element_num_bytes)),
-        in_options->element_num_bytes);
-  } else {
-    if (in_options->capacity_num_bytes % in_options->element_num_bytes != 0)
-      return MOJO_RESULT_INVALID_ARGUMENT;
-    out_options->capacity_num_bytes = in_options->capacity_num_bytes;
+            (kDefaultDataPipeCapacityBytes % out_options->element_num_bytes)),
+        out_options->element_num_bytes);
+    return MOJO_RESULT_OK;
   }
-  if (out_options->capacity_num_bytes > kMaxDataPipeCapacityBytes)
+  if (in_options->capacity_num_bytes % out_options->element_num_bytes != 0)
+    return MOJO_RESULT_INVALID_ARGUMENT;
+  if (in_options->capacity_num_bytes > kMaxDataPipeCapacityBytes)
     return MOJO_RESULT_RESOURCE_EXHAUSTED;
+  out_options->capacity_num_bytes = in_options->capacity_num_bytes;
 
   return MOJO_RESULT_OK;
 }
diff --git a/mojo/system/data_pipe_unittest.cc b/mojo/system/data_pipe_unittest.cc
index d933e55..abcaca2 100644
--- a/mojo/system/data_pipe_unittest.cc
+++ b/mojo/system/data_pipe_unittest.cc
@@ -4,9 +4,12 @@
 
 #include "mojo/system/data_pipe.h"
 
+#include <stddef.h>
+
 #include <limits>
 
 #include "base/basictypes.h"
+#include "mojo/system/constants.h"
 #include "testing/gtest/include/gtest/gtest.h"
 
 namespace mojo {
@@ -28,7 +31,7 @@ void RevalidateOptions(const MojoCreateDataPipeOptions& validated_options) {
             validated_options.capacity_num_bytes %
                 validated_options.element_num_bytes);
 
-  MojoCreateDataPipeOptions revalidated_options = { 0 };
+  MojoCreateDataPipeOptions revalidated_options = {};
   EXPECT_EQ(MOJO_RESULT_OK,
             DataPipe::ValidateOptions(&validated_options,
                                       &revalidated_options));
@@ -40,14 +43,37 @@ void RevalidateOptions(const MojoCreateDataPipeOptions& validated_options) {
   EXPECT_EQ(validated_options.flags, revalidated_options.flags);
 }
 
+// Checks that a default-computed capacity is correct. (Does not duplicate the
+// checks done by |RevalidateOptions()|.)
+void CheckDefaultCapacity(const MojoCreateDataPipeOptions& validated_options) {
+  EXPECT_LE(validated_options.capacity_num_bytes,
+            kDefaultDataPipeCapacityBytes);
+  EXPECT_GT(validated_options.capacity_num_bytes +
+                validated_options.element_num_bytes,
+            kDefaultDataPipeCapacityBytes);
+}
+
 // Tests valid inputs to |ValidateOptions()|.
 TEST(DataPipeTest, ValidateOptionsValidInputs) {
   // Default options.
   {
-    MojoCreateDataPipeOptions validated_options = { 0 };
+    MojoCreateDataPipeOptions validated_options = {};
     EXPECT_EQ(MOJO_RESULT_OK,
               DataPipe::ValidateOptions(NULL, &validated_options));
     RevalidateOptions(validated_options);
+    CheckDefaultCapacity(validated_options);
+  }
+
+  // Size member, but nothing beyond.
+  {
+    MojoCreateDataPipeOptions options = {
+      offsetof(MojoCreateDataPipeOptions, flags)  // |struct_size|.
+    };
+    MojoCreateDataPipeOptions validated_options = {};
+    EXPECT_EQ(MOJO_RESULT_OK,
+              DataPipe::ValidateOptions(&options, &validated_options));
+    RevalidateOptions(validated_options);
+    CheckDefaultCapacity(validated_options);
   }
 
   // Different flags.
@@ -58,6 +84,21 @@ TEST(DataPipeTest, ValidateOptionsValidInputs) {
   for (size_t i = 0; i < arraysize(flags_values); i++) {
     const MojoCreateDataPipeOptionsFlags flags = flags_values[i];
 
+    // Flags member, but nothing beyond.
+    {
+      MojoCreateDataPipeOptions options = {
+        // |struct_size|.
+        offsetof(MojoCreateDataPipeOptions, element_num_bytes),
+        flags  // |flags|.
+      };
+      MojoCreateDataPipeOptions validated_options = {};
+      EXPECT_EQ(MOJO_RESULT_OK,
+                DataPipe::ValidateOptions(&options, &validated_options));
+      RevalidateOptions(validated_options);
+      EXPECT_EQ(options.flags, validated_options.flags);
+      CheckDefaultCapacity(validated_options);
+    }
+
     // Different capacities (size 1).
     for (uint32_t capacity = 1; capacity <= 100 * 1000 * 1000; capacity *= 10) {
       MojoCreateDataPipeOptions options = {
@@ -66,11 +107,16 @@ TEST(DataPipeTest, ValidateOptionsValidInputs) {
         1,  // |element_num_bytes|.
         capacity  // |capacity_num_bytes|.
       };
-      MojoCreateDataPipeOptions validated_options = { 0 };
+      MojoCreateDataPipeOptions validated_options = {};
       EXPECT_EQ(MOJO_RESULT_OK,
                 DataPipe::ValidateOptions(&options, &validated_options))
           << capacity;
       RevalidateOptions(validated_options);
+      EXPECT_EQ(options.flags, validated_options.flags);
+      EXPECT_EQ(options.element_num_bytes,
+                validated_options.element_num_bytes);
+      EXPECT_EQ(options.capacity_num_bytes,
+                validated_options.capacity_num_bytes);
     }
 
     // Small sizes.
@@ -83,11 +129,16 @@ TEST(DataPipeTest, ValidateOptionsValidInputs) {
           size,  // |element_num_bytes|.
           size * elements  // |capacity_num_bytes|.
         };
-        MojoCreateDataPipeOptions validated_options = { 0 };
+        MojoCreateDataPipeOptions validated_options = {};
         EXPECT_EQ(MOJO_RESULT_OK,
                   DataPipe::ValidateOptions(&options, &validated_options))
             << size << ", " << elements;
         RevalidateOptions(validated_options);
+        EXPECT_EQ(options.flags, validated_options.flags);
+        EXPECT_EQ(options.element_num_bytes,
+                  validated_options.element_num_bytes);
+        EXPECT_EQ(options.capacity_num_bytes,
+                  validated_options.capacity_num_bytes);
       }
 
       // Default capacity.
@@ -98,11 +149,34 @@ TEST(DataPipeTest, ValidateOptionsValidInputs) {
           size,  // |element_num_bytes|.
           0  // |capacity_num_bytes|.
         };
-        MojoCreateDataPipeOptions validated_options = { 0 };
+        MojoCreateDataPipeOptions validated_options = {};
         EXPECT_EQ(MOJO_RESULT_OK,
                   DataPipe::ValidateOptions(&options, &validated_options))
             << size;
         RevalidateOptions(validated_options);
+        EXPECT_EQ(options.flags, validated_options.flags);
+        EXPECT_EQ(options.element_num_bytes,
+                  validated_options.element_num_bytes);
+        CheckDefaultCapacity(validated_options);
+      }
+
+      // No capacity field.
+      {
+        MojoCreateDataPipeOptions options = {
+          // |struct_size|.
+          offsetof(MojoCreateDataPipeOptions, capacity_num_bytes),
+          flags,  // |flags|.
+          size  // |element_num_bytes|.
+        };
+        MojoCreateDataPipeOptions validated_options = {};
+        EXPECT_EQ(MOJO_RESULT_OK,
+                  DataPipe::ValidateOptions(&options, &validated_options))
+            << size;
+        RevalidateOptions(validated_options);
+        EXPECT_EQ(options.flags, validated_options.flags);
+        EXPECT_EQ(options.element_num_bytes,
+                  validated_options.element_num_bytes);
+        CheckDefaultCapacity(validated_options);
       }
     }
 
@@ -116,11 +190,16 @@ TEST(DataPipeTest, ValidateOptionsValidInputs) {
           size,  // |element_num_bytes|.
           1000 * size  // |capacity_num_bytes|.
         };
-        MojoCreateDataPipeOptions validated_options = { 0 };
+        MojoCreateDataPipeOptions validated_options = {};
         EXPECT_EQ(MOJO_RESULT_OK,
                   DataPipe::ValidateOptions(&options, &validated_options))
             << size;
         RevalidateOptions(validated_options);
+        EXPECT_EQ(options.flags, validated_options.flags);
+        EXPECT_EQ(options.element_num_bytes,
+                  validated_options.element_num_bytes);
+        EXPECT_EQ(options.capacity_num_bytes,
+                  validated_options.capacity_num_bytes);
       }
 
       // Default capacity.
@@ -131,11 +210,34 @@ TEST(DataPipeTest, ValidateOptionsValidInputs) {
           size,  // |element_num_bytes|.
           0  // |capacity_num_bytes|.
         };
-        MojoCreateDataPipeOptions validated_options = { 0 };
+        MojoCreateDataPipeOptions validated_options = {};
         EXPECT_EQ(MOJO_RESULT_OK,
                   DataPipe::ValidateOptions(&options, &validated_options))
             << size;
         RevalidateOptions(validated_options);
+        EXPECT_EQ(options.flags, validated_options.flags);
+        EXPECT_EQ(options.element_num_bytes,
+                  validated_options.element_num_bytes);
+        CheckDefaultCapacity(validated_options);
+      }
+
+      // No capacity field.
+      {
+        MojoCreateDataPipeOptions options = {
+          // |struct_size|.
+          offsetof(MojoCreateDataPipeOptions, capacity_num_bytes),
+          flags,  // |flags|.
+          size  // |element_num_bytes|.
+        };
+        MojoCreateDataPipeOptions validated_options = {};
+        EXPECT_EQ(MOJO_RESULT_OK,
+                  DataPipe::ValidateOptions(&options, &validated_options))
+            << size;
+        RevalidateOptions(validated_options);
+        EXPECT_EQ(options.flags, validated_options.flags);
+        EXPECT_EQ(options.element_num_bytes,
+                  validated_options.element_num_bytes);
+        CheckDefaultCapacity(validated_options);
       }
     }
   }
@@ -143,20 +245,31 @@ TEST(DataPipeTest, ValidateOptionsValidInputs) {
 
 TEST(DataPipeTest, ValidateOptionsInvalidInputs) {
   // Invalid |struct_size|.
-  // Note: If/when we extend |MojoCreateDataPipeOptions|, this will have to be
-  // updated.
-  for (uint32_t struct_size = 0; struct_size < kSizeOfOptions; struct_size++) {
+  {
     MojoCreateDataPipeOptions options = {
-      struct_size,  // |struct_size|.
+      1,  // |struct_size|.
       MOJO_CREATE_DATA_PIPE_OPTIONS_FLAG_NONE,  // |flags|.
       1,  // |element_num_bytes|.
-      1000  // |capacity_num_bytes|.
+      0  // |capacity_num_bytes|.
     };
-    MojoCreateDataPipeOptions unused = { 0 };
+    MojoCreateDataPipeOptions unused;
     EXPECT_EQ(MOJO_RESULT_INVALID_ARGUMENT,
               DataPipe::ValidateOptions(&options, &unused));
   }
 
+  // Unknown |flags|.
+  {
+    MojoCreateDataPipeOptions options = {
+      kSizeOfOptions,  // |struct_size|.
+      ~0u,  // |flags|.
+      1,  // |element_num_bytes|.
+      0  // |capacity_num_bytes|.
+    };
+    MojoCreateDataPipeOptions unused;
+    EXPECT_EQ(MOJO_RESULT_UNIMPLEMENTED,
+              DataPipe::ValidateOptions(&options, &unused));
+  }
+
   // Invalid |element_num_bytes|.
   {
     MojoCreateDataPipeOptions options = {
@@ -165,7 +278,7 @@ TEST(DataPipeTest, ValidateOptionsInvalidInputs) {
       0,  // |element_num_bytes|.
       1000  // |capacity_num_bytes|.
     };
-    MojoCreateDataPipeOptions unused = { 0 };
+    MojoCreateDataPipeOptions unused;
     EXPECT_EQ(MOJO_RESULT_INVALID_ARGUMENT,
               DataPipe::ValidateOptions(&options, &unused));
   }
@@ -177,7 +290,7 @@ TEST(DataPipeTest, ValidateOptionsInvalidInputs) {
       std::numeric_limits<uint32_t>::max(),  // |element_num_bytes|.
       std::numeric_limits<uint32_t>::max()  // |capacity_num_bytes|.
     };
-    MojoCreateDataPipeOptions unused = { 0 };
+    MojoCreateDataPipeOptions unused;
     EXPECT_EQ(MOJO_RESULT_RESOURCE_EXHAUSTED,
               DataPipe::ValidateOptions(&options, &unused));
   }
@@ -188,7 +301,7 @@ TEST(DataPipeTest, ValidateOptionsInvalidInputs) {
       std::numeric_limits<uint32_t>::max() - 1000,  // |element_num_bytes|.
       std::numeric_limits<uint32_t>::max() - 1000  // |capacity_num_bytes|.
     };
-    MojoCreateDataPipeOptions unused = { 0 };
+    MojoCreateDataPipeOptions unused;
     EXPECT_EQ(MOJO_RESULT_RESOURCE_EXHAUSTED,
               DataPipe::ValidateOptions(&options, &unused));
   }
@@ -201,7 +314,7 @@ TEST(DataPipeTest, ValidateOptionsInvalidInputs) {
       2,  // |element_num_bytes|.
       1  // |capacity_num_bytes|.
     };
-    MojoCreateDataPipeOptions unused = { 0 };
+    MojoCreateDataPipeOptions unused;
     EXPECT_EQ(MOJO_RESULT_INVALID_ARGUMENT,
               DataPipe::ValidateOptions(&options, &unused));
   }
@@ -212,7 +325,7 @@ TEST(DataPipeTest, ValidateOptionsInvalidInputs) {
       2,  // |element_num_bytes|.
       111  // |capacity_num_bytes|.
     };
-    MojoCreateDataPipeOptions unused = { 0 };
+    MojoCreateDataPipeOptions unused;
     EXPECT_EQ(MOJO_RESULT_INVALID_ARGUMENT,
               DataPipe::ValidateOptions(&options, &unused));
   }
@@ -223,7 +336,7 @@ TEST(DataPipeTest, ValidateOptionsInvalidInputs) {
       5,  // |element_num_bytes|.
       104  // |capacity_num_bytes|.
     };
-    MojoCreateDataPipeOptions unused = { 0 };
+    MojoCreateDataPipeOptions unused;
     EXPECT_EQ(MOJO_RESULT_INVALID_ARGUMENT,
               DataPipe::ValidateOptions(&options, &unused));
   }
@@ -235,7 +348,7 @@ TEST(DataPipeTest, ValidateOptionsInvalidInputs) {
       8,  // |element_num_bytes|.
       0xffff0000  // |capacity_num_bytes|.
     };
-    MojoCreateDataPipeOptions unused = { 0 };
+    MojoCreateDataPipeOptions unused;
     EXPECT_EQ(MOJO_RESULT_RESOURCE_EXHAUSTED,
               DataPipe::ValidateOptions(&options, &unused));
   }
diff --git a/mojo/system/memory.cc b/mojo/system/memory.cc
index 498ca0b..cee7e05 100644
--- a/mojo/system/memory.cc
+++ b/mojo/system/memory.cc
@@ -19,9 +19,9 @@ bool IsAligned(const void* pointer) {
   return reinterpret_cast<uintptr_t>(pointer) % alignment == 0;
 }
 
-#if defined(COMPILER_MSVC) && defined(ARCH_CPU_32_BITS)
 // MSVS (2010, 2013) sometimes (on the stack) aligns, e.g., |int64_t|s (for
 // which |__alignof(int64_t)| is 8) to 4-byte boundaries. http://goo.gl/Y2n56T
+#if defined(COMPILER_MSVC) && defined(ARCH_CPU_32_BITS)
 template <>
 bool IsAligned<8>(const void* pointer) {
   return reinterpret_cast<uintptr_t>(pointer) % 4 == 0;
@@ -37,12 +37,26 @@ bool MOJO_SYSTEM_IMPL_EXPORT VerifyUserPointerHelper(const void* pointer) {
 }
 
 // Explicitly instantiate the sizes we need. Add instantiations as needed.
-template MOJO_SYSTEM_IMPL_EXPORT bool VerifyUserPointerHelper<1, 1>(
+template bool MOJO_SYSTEM_IMPL_EXPORT VerifyUserPointerHelper<1, 1>(
     const void*);
-template MOJO_SYSTEM_IMPL_EXPORT bool VerifyUserPointerHelper<4, 4>(
+template bool MOJO_SYSTEM_IMPL_EXPORT VerifyUserPointerHelper<4, 4>(
     const void*);
-template MOJO_SYSTEM_IMPL_EXPORT bool VerifyUserPointerHelper<8, 8>(
+template bool MOJO_SYSTEM_IMPL_EXPORT VerifyUserPointerHelper<8, 8>(
     const void*);
+// Notwithstanding the comments above about MSVS, whenever we expect an
+// alignment of 8 for something of size 4, it's due to an explicit (e.g.,
+// #pragma align) alignment specification, which MSVS *does* respect. We want
+// this in particular to check that various Options structs are aligned.
+#if defined(COMPILER_MSVC) && defined(ARCH_CPU_32_BITS)
+template <>
+bool MOJO_SYSTEM_IMPL_EXPORT VerifyUserPointerHelper<4, 8>(
+    const void* pointer) {
+  return !!pointer && reinterpret_cast<uintptr_t>(pointer) % 8 == 0;
+}
+#else
+template MOJO_SYSTEM_IMPL_EXPORT bool VerifyUserPointerHelper<4, 8>(
+    const void*);
+#endif
 
 template <size_t size, size_t alignment>
 bool VerifyUserPointerWithCountHelper(const void* pointer, size_t count) {
@@ -56,11 +70,11 @@ bool VerifyUserPointerWithCountHelper(const void* pointer, size_t count) {
 }
 
 // Explicitly instantiate the sizes we need. Add instantiations as needed.
-template MOJO_SYSTEM_IMPL_EXPORT bool VerifyUserPointerWithCountHelper<1, 1>(
+template bool MOJO_SYSTEM_IMPL_EXPORT VerifyUserPointerWithCountHelper<1, 1>(
     const void*, size_t);
-template MOJO_SYSTEM_IMPL_EXPORT bool VerifyUserPointerWithCountHelper<4, 4>(
+template bool MOJO_SYSTEM_IMPL_EXPORT VerifyUserPointerWithCountHelper<4, 4>(
     const void*, size_t);
-template MOJO_SYSTEM_IMPL_EXPORT bool VerifyUserPointerWithCountHelper<8, 8>(
+template bool MOJO_SYSTEM_IMPL_EXPORT VerifyUserPointerWithCountHelper<8, 8>(
     const void*, size_t);
 
 }  // nameespace internal
@@ -74,11 +88,11 @@ bool VerifyUserPointerWithSize(const void* pointer, size_t size) {
 }
 
 // Explicitly instantiate the alignments we need. Add instantiations as needed.
-template MOJO_SYSTEM_IMPL_EXPORT bool VerifyUserPointerWithSize<1>(const void*,
+template bool MOJO_SYSTEM_IMPL_EXPORT VerifyUserPointerWithSize<1>(const void*,
                                                                    size_t);
-template MOJO_SYSTEM_IMPL_EXPORT bool VerifyUserPointerWithSize<4>(const void*,
+template bool MOJO_SYSTEM_IMPL_EXPORT VerifyUserPointerWithSize<4>(const void*,
                                                                    size_t);
-template MOJO_SYSTEM_IMPL_EXPORT bool VerifyUserPointerWithSize<8>(const void*,
+template bool MOJO_SYSTEM_IMPL_EXPORT VerifyUserPointerWithSize<8>(const void*,
                                                                    size_t);
 
 }  // namespace system
diff --git a/mojo/system/memory.h b/mojo/system/memory.h
index 21c36ea..96f800e 100644
--- a/mojo/system/memory.h
+++ b/mojo/system/memory.h
@@ -18,6 +18,7 @@ namespace internal {
 template <size_t size, size_t alignment>
 bool MOJO_SYSTEM_IMPL_EXPORT VerifyUserPointerHelper(const void* pointer);
 
+// Note: This is also used by options_validation.h.
 template <size_t size, size_t alignment>
 bool MOJO_SYSTEM_IMPL_EXPORT VerifyUserPointerWithCountHelper(
     const void* pointer,
diff --git a/mojo/system/options_validation.h b/mojo/system/options_validation.h
new file mode 100644
index 0000000..8ee6a69
--- /dev/null
+++ b/mojo/system/options_validation.h
@@ -0,0 +1,90 @@
+// Copyright 2014 The Chromium Authors. All rights reserved.
+// Use of this source code is governed by a BSD-style license that can be
+// found in the LICENSE file.
+
+// Functions to help with verifying various |Mojo...Options| structs from the
+// (public, C) API. These are "extensible" structs, which all have |struct_size|
+// as their first member. All fields (other than |struct_size|) are optional,
+// but any |flags| specified must be known to the system (otherwise, an error of
+// |MOJO_RESULT_UNIMPLEMENTED| should be returned).
+
+#ifndef MOJO_SYSTEM_OPTIONS_VALIDATION_H_
+#define MOJO_SYSTEM_OPTIONS_VALIDATION_H_
+
+#include <stddef.h>
+#include <stdint.h>
+
+#include "base/macros.h"
+#include "mojo/system/memory.h"
+#include "mojo/system/system_impl_export.h"
+
+namespace mojo {
+namespace system {
+
+// Checks that |buffer| appears to contain a valid Options struct, namely
+// properly aligned and with a |struct_size| field (which must the first field
+// of the struct and be a |uint32_t|) containing a plausible size.
+template <class Options>
+bool IsOptionsStructPointerAndSizeValid(const void* buffer) {
+  COMPILE_ASSERT(offsetof(Options, struct_size) == 0,
+                 Options_struct_size_not_first_member);
+  // TODO(vtl): With C++11, use |sizeof(Options::struct_size)| instead.
+  COMPILE_ASSERT(sizeof(static_cast<const Options*>(buffer)->struct_size) ==
+                     sizeof(uint32_t),
+                 Options_struct_size_not_32_bits);
+
+  // Note: Use |MOJO_ALIGNOF()| here to match the exact macro used in the
+  // declaration of Options structs.
+  if (!internal::VerifyUserPointerHelper<sizeof(uint32_t),
+                                         MOJO_ALIGNOF(Options)>(buffer))
+    return false;
+
+  return static_cast<const Options*>(buffer)->struct_size >= sizeof(uint32_t);
+}
+
+// Checks that the Options struct in |buffer| has a member with the given offset
+// and size. This may be called only if |IsOptionsStructPointerAndSizeValid()|
+// returned true.
+//
+// You may want to use the macro |HAS_OPTIONS_STRUCT_MEMBER()| instead.
+template <class Options, size_t offset, size_t size>
+bool HasOptionsStructMember(const void* buffer) {
+  // We assume that |offset| and |size| are reasonable, since they should come
+  // from |offsetof(Options, some_member)| and |sizeof(Options::some_member)|,
+  // respectively.
+  return static_cast<const Options*>(buffer)->struct_size >=
+      offset + size;
+}
+
+// Macro to invoke |HasOptionsStructMember()| parametrized by member name
+// instead of offset and size.
+//
+// (We can't just give |HasOptionsStructMember()| a member pointer template
+// argument instead, since there's no good/strictly-correct way to get an offset
+// from that.)
+//
+// TODO(vtl): With C++11, use |sizeof(Options::member)| instead.
+#define HAS_OPTIONS_STRUCT_MEMBER(Options, member, buffer) \
+    (HasOptionsStructMember< \
+        Options, \
+        offsetof(Options, member), \
+        sizeof(static_cast<const Options*>(buffer)->member)>(buffer))
+
+// Checks that the (standard) |flags| member consists of only known flags. This
+// should only be called if |HAS_OPTIONS_STRUCT_MEMBER()| returned true for the
+// |flags| field.
+//
+// The rationale for *not* ignoring these flags is that the caller should have a
+// way of specifying that certain options not be ignored. E.g., one may have a
+// |MOJO_..._OPTIONS_FLAG_DONT_IGNORE_FOO| flag and a |foo| member; if the flag
+// is set, it will guarantee that the version of the system knows about the
+// |foo| member (and won't ignore it).
+template <class Options>
+bool AreOptionsFlagsAllKnown(const void* buffer, uint32_t known_flags) {
+  return (static_cast<const Options*>(buffer)->flags & ~known_flags) == 0;
+}
+
+}  // namespace system
+}  // namespace mojo
+
+#endif  // MOJO_SYSTEM_OPTIONS_VALIDATION_H_
diff --git a/mojo/system/options_validation_unittest.cc b/mojo/system/options_validation_unittest.cc
new file mode 100644
index 0000000..27bb4ba
--- /dev/null
+++ b/mojo/system/options_validation_unittest.cc
@@ -0,0 +1,117 @@
+// Copyright 2014 The Chromium Authors. All rights reserved.
+// Use of this source code is governed by a BSD-style license that can be
+// found in the LICENSE file.
+
+#include "mojo/system/options_validation.h"
+
+#include <stddef.h>
+#include <stdint.h>
+
+#include "mojo/public/c/system/macros.h"
+#include "testing/gtest/include/gtest/gtest.h"
+
+namespace mojo {
+namespace system {
+namespace {
+
+// Declare a test options struct just as we do in actual public headers.
+
+typedef uint32_t TestOptionsFlags;
+
+MOJO_COMPILE_ASSERT(MOJO_ALIGNOF(int64_t) == 8, int64_t_has_weird_alignment);
+struct MOJO_ALIGNAS(8) TestOptions {
+  uint32_t struct_size;
+  TestOptionsFlags flags;
+  uint32_t member1;
+  uint32_t member2;
+};
+MOJO_COMPILE_ASSERT(sizeof(TestOptions) == 16, TestOptions_has_wrong_size);
+
+TEST(OptionsValidationTest, Valid) {
+  const TestOptions kOptions1 = {static_cast<uint32_t>(sizeof(TestOptions))};
+
+  EXPECT_TRUE(IsOptionsStructPointerAndSizeValid<TestOptions>(&kOptions1));
+  EXPECT_TRUE(HAS_OPTIONS_STRUCT_MEMBER(TestOptions, flags, &kOptions1));
+  EXPECT_TRUE(HAS_OPTIONS_STRUCT_MEMBER(TestOptions, member1, &kOptions1));
+  EXPECT_TRUE(HAS_OPTIONS_STRUCT_MEMBER(TestOptions, member2, &kOptions1));
+
+  const TestOptions kOptions2 = {offsetof(TestOptions, struct_size) +
+                                     sizeof(uint32_t)};
+  EXPECT_TRUE(IsOptionsStructPointerAndSizeValid<TestOptions>(&kOptions2));
+  EXPECT_FALSE(HAS_OPTIONS_STRUCT_MEMBER(TestOptions, flags, &kOptions2));
+  EXPECT_FALSE(HAS_OPTIONS_STRUCT_MEMBER(TestOptions, member1, &kOptions2));
+  EXPECT_FALSE(HAS_OPTIONS_STRUCT_MEMBER(TestOptions, member2, &kOptions2));
+
+  const TestOptions kOptions3 = {offsetof(TestOptions, flags) +
+                                     sizeof(uint32_t)};
+  EXPECT_TRUE(IsOptionsStructPointerAndSizeValid<TestOptions>(&kOptions3));
+  EXPECT_TRUE(HAS_OPTIONS_STRUCT_MEMBER(TestOptions, flags, &kOptions3));
+  EXPECT_FALSE(HAS_OPTIONS_STRUCT_MEMBER(TestOptions, member1, &kOptions3));
+  EXPECT_FALSE(HAS_OPTIONS_STRUCT_MEMBER(TestOptions, member2, &kOptions3));
+
+  MOJO_ALIGNAS(8) char buf[sizeof(TestOptions) + 100] = {};
+  TestOptions* options = reinterpret_cast<TestOptions*>(buf);
+  options->struct_size = sizeof(TestOptions) + 1;
+  EXPECT_TRUE(IsOptionsStructPointerAndSizeValid<TestOptions>(options));
+  EXPECT_TRUE(HAS_OPTIONS_STRUCT_MEMBER(TestOptions, flags, options));
+  EXPECT_TRUE(HAS_OPTIONS_STRUCT_MEMBER(TestOptions, member1, options));
+  EXPECT_TRUE(HAS_OPTIONS_STRUCT_MEMBER(TestOptions, member2, options));
+
+  options->struct_size = sizeof(TestOptions) + 4;
+  EXPECT_TRUE(IsOptionsStructPointerAndSizeValid<TestOptions>(options));
+  EXPECT_TRUE(HAS_OPTIONS_STRUCT_MEMBER(TestOptions, flags, options));
+  EXPECT_TRUE(HAS_OPTIONS_STRUCT_MEMBER(TestOptions, member1, options));
+  EXPECT_TRUE(HAS_OPTIONS_STRUCT_MEMBER(TestOptions, member2, options));
+}
+
+TEST(OptionsValidationTest, Invalid) {
+  // Null:
+  EXPECT_FALSE(IsOptionsStructPointerAndSizeValid<TestOptions>(NULL));
+
+  // Unaligned:
+  EXPECT_FALSE(IsOptionsStructPointerAndSizeValid<TestOptions>(
+                   reinterpret_cast<const void*>(1)));
+  EXPECT_FALSE(IsOptionsStructPointerAndSizeValid<TestOptions>(
+                   reinterpret_cast<const void*>(4)));
+
+  // Size too small:
+  for (size_t i = 0; i < sizeof(uint32_t); i++) {
+    TestOptions options = {static_cast<uint32_t>(i)};
+    EXPECT_FALSE(IsOptionsStructPointerAndSizeValid<TestOptions>(&options))
+        << i;
+  }
+}
+
+TEST(OptionsValidationTest, CheckFlags) {
+  TestOptions kOptions1 = {static_cast<uint32_t>(sizeof(TestOptions)), 0};
+  EXPECT_TRUE(AreOptionsFlagsAllKnown<TestOptions>(&kOptions1, 0u));
+  EXPECT_TRUE(AreOptionsFlagsAllKnown<TestOptions>(&kOptions1, 1u));
+  EXPECT_TRUE(AreOptionsFlagsAllKnown<TestOptions>(&kOptions1, 3u));
+  EXPECT_TRUE(AreOptionsFlagsAllKnown<TestOptions>(&kOptions1, 7u));
+  EXPECT_TRUE(AreOptionsFlagsAllKnown<TestOptions>(&kOptions1, ~0u));
+
+  TestOptions kOptions2 = {static_cast<uint32_t>(sizeof(TestOptions)), 1};
+  EXPECT_FALSE(AreOptionsFlagsAllKnown<TestOptions>(&kOptions2, 0u));
+  EXPECT_TRUE(AreOptionsFlagsAllKnown<TestOptions>(&kOptions2, 1u));
+  EXPECT_TRUE(AreOptionsFlagsAllKnown<TestOptions>(&kOptions2, 3u));
+  EXPECT_TRUE(AreOptionsFlagsAllKnown<TestOptions>(&kOptions2, 7u));
+  EXPECT_TRUE(AreOptionsFlagsAllKnown<TestOptions>(&kOptions2, ~0u));
+
+  TestOptions kOptions3 = {static_cast<uint32_t>(sizeof(TestOptions)), 2};
+  EXPECT_FALSE(AreOptionsFlagsAllKnown<TestOptions>(&kOptions3, 0u));
+  EXPECT_FALSE(AreOptionsFlagsAllKnown<TestOptions>(&kOptions3, 1u));
+  EXPECT_TRUE(AreOptionsFlagsAllKnown<TestOptions>(&kOptions3, 3u));
+  EXPECT_TRUE(AreOptionsFlagsAllKnown<TestOptions>(&kOptions3, 7u));
+  EXPECT_TRUE(AreOptionsFlagsAllKnown<TestOptions>(&kOptions3, ~0u));
+
+  TestOptions kOptions4 = {static_cast<uint32_t>(sizeof(TestOptions)), 5};
+  EXPECT_FALSE(AreOptionsFlagsAllKnown<TestOptions>(&kOptions4, 0u));
+  EXPECT_FALSE(AreOptionsFlagsAllKnown<TestOptions>(&kOptions4, 1u));
+  EXPECT_FALSE(AreOptionsFlagsAllKnown<TestOptions>(&kOptions4, 3u));
+  EXPECT_TRUE(AreOptionsFlagsAllKnown<TestOptions>(&kOptions4, 7u));
+  EXPECT_TRUE(AreOptionsFlagsAllKnown<TestOptions>(&kOptions4, ~0u));
+}
+
+}  // namespace
+}  // namespace system
+}  // namespace mojo
diff --git a/mojo/system/shared_buffer_dispatcher.cc b/mojo/system/shared_buffer_dispatcher.cc
index 841c659..e46c230 100644
--- a/mojo/system/shared_buffer_dispatcher.cc
+++ b/mojo/system/shared_buffer_dispatcher.cc
@@ -11,6 +11,7 @@
 #include "mojo/public/c/system/macros.h"
 #include "mojo/system/constants.h"
 #include "mojo/system/memory.h"
+#include "mojo/system/options_validation.h"
 #include "mojo/system/raw_shared_buffer.h"
 
 namespace mojo {
@@ -33,16 +34,21 @@ MojoResult SharedBufferDispatcher::ValidateOptions(
     static_cast<uint32_t>(sizeof(MojoCreateSharedBufferOptions)),
     MOJO_CREATE_SHARED_BUFFER_OPTIONS_FLAG_NONE
   };
-  if (!in_options) {
-    *out_options = kDefaultOptions;
+  *out_options = kDefaultOptions;
+
+  if (!in_options)
     return MOJO_RESULT_OK;
-  }
 
-  if (in_options->struct_size < sizeof(*in_options))
+  if (!IsOptionsStructPointerAndSizeValid<MojoCreateSharedBufferOptions>(
+          in_options))
     return MOJO_RESULT_INVALID_ARGUMENT;
-  out_options->struct_size = static_cast<uint32_t>(sizeof(*out_options));
 
-  // All flags are okay (unrecognized flags will be ignored).
+  if (!HAS_OPTIONS_STRUCT_MEMBER(MojoCreateSharedBufferOptions, flags,
+                                 in_options))
+    return MOJO_RESULT_OK;
+  if (!AreOptionsFlagsAllKnown<MojoCreateSharedBufferOptions>(
+          in_options, MOJO_CREATE_DATA_PIPE_OPTIONS_FLAG_NONE))
+    return MOJO_RESULT_UNIMPLEMENTED;
   out_options->flags = in_options->flags;
 
   return MOJO_RESULT_OK;
diff --git a/mojo/system/shared_buffer_dispatcher_unittest.cc b/mojo/system/shared_buffer_dispatcher_unittest.cc
index 36cee14..0c44554 100644
--- a/mojo/system/shared_buffer_dispatcher_unittest.cc
+++ b/mojo/system/shared_buffer_dispatcher_unittest.cc
@@ -30,7 +30,7 @@ void RevalidateOptions(const MojoCreateSharedBufferOptions& validated_options) {
   EXPECT_EQ(kSizeOfOptions, validated_options.struct_size);
   // Nothing to check for flags.
 
-  MojoCreateSharedBufferOptions revalidated_options = { 0 };
+  MojoCreateSharedBufferOptions revalidated_options = {};
   EXPECT_EQ(MOJO_RESULT_OK,
             SharedBufferDispatcher::ValidateOptions(&validated_options,
                                                     &revalidated_options));
@@ -42,7 +42,7 @@ void RevalidateOptions(const MojoCreateSharedBufferOptions& validated_options) {
 TEST(SharedBufferDispatcherTest, ValidateOptionsValidInputs) {
   // Default options.
   {
-    MojoCreateSharedBufferOptions validated_options = { 0 };
+    MojoCreateSharedBufferOptions validated_options = {};
     EXPECT_EQ(MOJO_RESULT_OK,
               SharedBufferDispatcher::ValidateOptions(NULL,
                                                       &validated_options));
@@ -62,33 +62,43 @@ TEST(SharedBufferDispatcherTest, ValidateOptionsValidInputs) {
         kSizeOfOptions,  // |struct_size|.
         flags  // |flags|.
       };
-      MojoCreateSharedBufferOptions validated_options = { 0 };
+      MojoCreateSharedBufferOptions validated_options = {};
       EXPECT_EQ(MOJO_RESULT_OK,
                 SharedBufferDispatcher::ValidateOptions(&options,
                                                         &validated_options))
           << capacity;
       RevalidateOptions(validated_options);
+      EXPECT_EQ(options.flags, validated_options.flags);
     }
   }
 }
 
 TEST(SharedBufferDispatcherTest, ValidateOptionsInvalidInputs) {
   // Invalid |struct_size|.
-  // Note: If/when we extend |MojoCreateSharedBufferOptions|, this will have to
-  // be updated.
-  for (uint32_t struct_size = 0; struct_size < kSizeOfOptions; struct_size++) {
+  {
     MojoCreateSharedBufferOptions options = {
-      struct_size,  // |struct_size|.
+      1,  // |struct_size|.
       MOJO_CREATE_SHARED_BUFFER_OPTIONS_FLAG_NONE  // |flags|.
     };
-    MojoCreateSharedBufferOptions unused = { 0 };
+    MojoCreateSharedBufferOptions unused;
     EXPECT_EQ(MOJO_RESULT_INVALID_ARGUMENT,
               SharedBufferDispatcher::ValidateOptions(&options, &unused));
   }
+
+  // Unknown |flags|.
+  {
+    MojoCreateSharedBufferOptions options = {
+      kSizeOfOptions,  // |struct_size|.
+      ~0u  // |flags|.
+    };
+    MojoCreateSharedBufferOptions unused;
+    EXPECT_EQ(MOJO_RESULT_UNIMPLEMENTED,
+              SharedBufferDispatcher::ValidateOptions(&options, &unused));
+  }
 }
 
 TEST(SharedBufferDispatcherTest, CreateAndMapBuffer) {
-  MojoCreateSharedBufferOptions validated_options = { 0 };
+  MojoCreateSharedBufferOptions validated_options = {};
   EXPECT_EQ(MOJO_RESULT_OK,
             SharedBufferDispatcher::ValidateOptions(NULL,
                                                     &validated_options));
@@ -129,7 +139,7 @@ TEST(SharedBufferDispatcherTest, CreateAndMapBuffer) {
 }
 
 TEST(SharedBufferDispatcher, DuplicateBufferHandle) {
-  MojoCreateSharedBufferOptions validated_options = { 0 };
+  MojoCreateSharedBufferOptions validated_options = {};
   EXPECT_EQ(MOJO_RESULT_OK,
             SharedBufferDispatcher::ValidateOptions(NULL,
                                                     &validated_options));
@@ -168,7 +178,7 @@ TEST(SharedBufferDispatcher, DuplicateBufferHandle) {
 // TODO(vtl): Test |DuplicateBufferHandle()| with non-null (valid) |options|.
 
 TEST(SharedBufferDispatcherTest, CreateInvalidNumBytes) {
-  MojoCreateSharedBufferOptions validated_options = { 0 };
+  MojoCreateSharedBufferOptions validated_options = {};
   EXPECT_EQ(MOJO_RESULT_OK,
             SharedBufferDispatcher::ValidateOptions(NULL,
                                                     &validated_options));
@@ -188,7 +198,7 @@ TEST(SharedBufferDispatcherTest, CreateInvalidNumBytes) {
 }
 
 TEST(SharedBufferDispatcherTest, MapBufferInvalidArguments) {
-  MojoCreateSharedBufferOptions validated_options = { 0 };
+  MojoCreateSharedBufferOptions validated_options = {};
   EXPECT_EQ(MOJO_RESULT_OK,
             SharedBufferDispatcher::ValidateOptions(NULL,
                                                     &validated_options));