Mojo: Convert MemoryTest.Valid to the new user pointer handling (see r285350).

Ifdef out MemoryTest.Invalid until I convert it. This lets me remove the
old (now otherwise-unused) Verify...() functions.

Also delete UserPointerValue, etc.: It's easier to just add a
GetPointerValue() method to UserPointer.

R=darin@chromium.org

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

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

mojo/system/core.cc

@@ -500,9 +500,9 @@ MojoResult Core::MapBuffer(MojoHandle buffer_handle,
   return MOJO_RESULT_OK;
 }
 
-MojoResult Core::UnmapBuffer(UserPointerValue<void> buffer) {
+MojoResult Core::UnmapBuffer(UserPointer<void> buffer) {
   base::AutoLock locker(mapping_table_lock_);
-  return mapping_table_.RemoveMapping(buffer.GetValue());
+  return mapping_table_.RemoveMapping(buffer.GetPointerValue());
 }
 
 // Note: We allow |handles| to repeat the same handle multiple times, since

mojo/system/core.h

@@ -102,7 +102,7 @@ class MOJO_SYSTEM_IMPL_EXPORT Core {
                        uint64_t num_bytes,
                        UserPointer<void*> buffer,
                        MojoMapBufferFlags flags);
-  MojoResult UnmapBuffer(UserPointerValue<void> buffer);
+  MojoResult UnmapBuffer(UserPointer<void> buffer);
 
  private:
   friend bool internal::ShutdownCheckNoLeaks(Core*);

mojo/system/entrypoints.cc

@@ -13,7 +13,6 @@
 static mojo::system::Core* g_core = NULL;
 
 using mojo::system::MakeUserPointer;
-using mojo::system::MakeUserPointerValue;
 
 namespace mojo {
 namespace system {
@@ -183,7 +182,7 @@ MojoResult MojoMapBuffer(MojoHandle buffer_handle,
 }
 
 MojoResult MojoUnmapBuffer(void* buffer) {
-  return g_core->UnmapBuffer(MakeUserPointerValue(buffer));
+  return g_core->UnmapBuffer(MakeUserPointer(buffer));
 }
 
 }  // extern "C"

mojo/system/memory.cc

@@ -11,7 +11,6 @@
 
 namespace mojo {
 namespace system {
-
 namespace internal {
 
 template <size_t alignment>
@@ -33,14 +32,6 @@ void MOJO_SYSTEM_IMPL_EXPORT CheckUserPointer(const void* pointer) {
   CHECK(pointer && IsAligned<alignment>(pointer));
 }
 
-template <size_t size, size_t alignment>
-bool MOJO_SYSTEM_IMPL_EXPORT VerifyUserPointerHelper(const void* pointer) {
-  // TODO(vtl): If running in kernel mode, do a full verification. For now, just
-  // check that it's non-null and aligned. (A faster user mode implementation is
-  // also possible if this check is skipped.)
-  return !!pointer && IsAligned<alignment>(pointer);
-}
-
 // Explicitly instantiate the sizes we need. Add instantiations as needed.
 template void MOJO_SYSTEM_IMPL_EXPORT CheckUserPointer<1, 1>(
     const void*);
@@ -48,12 +39,6 @@ template void MOJO_SYSTEM_IMPL_EXPORT CheckUserPointer<4, 4>(
     const void*);
 template void MOJO_SYSTEM_IMPL_EXPORT CheckUserPointer<8, 8>(
     const void*);
-template bool MOJO_SYSTEM_IMPL_EXPORT VerifyUserPointerHelper<1, 1>(
-    const void*);
-template bool MOJO_SYSTEM_IMPL_EXPORT VerifyUserPointerHelper<4, 4>(
-    const void*);
-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
@@ -64,16 +49,9 @@ void MOJO_SYSTEM_IMPL_EXPORT CheckUserPointer<4, 8>(
     const void* pointer) {
   CHECK(pointer && reinterpret_cast<uintptr_t>(pointer) % 8 == 0);
 }
-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 void CheckUserPointer<4, 8>(
     const void*);
-template MOJO_SYSTEM_IMPL_EXPORT bool VerifyUserPointerHelper<4, 8>(
-    const void*);
 #endif
 
 template <size_t size, size_t alignment>
@@ -107,42 +85,6 @@ template void MOJO_SYSTEM_IMPL_EXPORT CheckUserPointerWithSize<4>(const void*,
 template void MOJO_SYSTEM_IMPL_EXPORT CheckUserPointerWithSize<8>(const void*,
                                                                   size_t);
 
-template <size_t size, size_t alignment>
-bool VerifyUserPointerWithCountHelper(const void* pointer, size_t count) {
-  if (count > std::numeric_limits<size_t>::max() / size)
-    return false;
-
-  // TODO(vtl): If running in kernel mode, do a full verification. For now, just
-  // check that it's non-null and aligned if |count| is nonzero. (A faster user
-  // mode implementation is also possible if this check is skipped.)
-  return count == 0 || (!!pointer && IsAligned<alignment>(pointer));
-}
-
-// Explicitly instantiate the sizes we need. Add instantiations as needed.
-template bool MOJO_SYSTEM_IMPL_EXPORT VerifyUserPointerWithCountHelper<1, 1>(
-    const void*, size_t);
-template bool MOJO_SYSTEM_IMPL_EXPORT VerifyUserPointerWithCountHelper<4, 4>(
-    const void*, size_t);
-template bool MOJO_SYSTEM_IMPL_EXPORT VerifyUserPointerWithCountHelper<8, 8>(
-    const void*, size_t);
-
 }  // namespace internal
-
-template <size_t alignment>
-bool VerifyUserPointerWithSize(const void* pointer, size_t size) {
-  // TODO(vtl): If running in kernel mode, do a full verification. For now, just
-  // check that it's non-null and aligned. (A faster user mode implementation is
-  // also possible if this check is skipped.)
-  return size == 0 || (!!pointer && internal::IsAligned<alignment>(pointer));
-}
-
-// Explicitly instantiate the alignments we need. Add instantiations as needed.
-template bool MOJO_SYSTEM_IMPL_EXPORT VerifyUserPointerWithSize<1>(const void*,
-                                                                   size_t);
-template bool MOJO_SYSTEM_IMPL_EXPORT VerifyUserPointerWithSize<4>(const void*,
-                                                                   size_t);
-template bool MOJO_SYSTEM_IMPL_EXPORT VerifyUserPointerWithSize<8>(const void*,
-                                                                   size_t);
-
 }  // namespace system
 }  // namespace mojo

mojo/system/memory.h

@@ -46,17 +46,6 @@ template <size_t alignment>
 void MOJO_SYSTEM_IMPL_EXPORT CheckUserPointerWithSize(const void* pointer,
                                                       size_t size);
 
-// TODO(vtl): Delete all the |Verify...()| things (and replace them with
-// |Check...()|.
-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,
-    size_t count);
-
 }  // namespace internal
 
 // Forward declarations so that they can be friended.
@@ -65,30 +54,6 @@ template <typename Type> class UserPointerWriter;
 template <typename Type> class UserPointerReaderWriter;
 template <class Options> class UserOptionsReader;
 
-// Verify (insofar as possible/necessary) that a |T| can be read from the user
-// |pointer|.
-template <typename T>
-bool VerifyUserPointer(const T* pointer) {
-  return internal::VerifyUserPointerHelper<sizeof(T), MOJO_ALIGNOF(T)>(pointer);
-}
-
-// Verify (insofar as possible/necessary) that |count| |T|s can be read from the
-// user |pointer|; |count| may be zero. (This is done carefully since |count *
-// sizeof(T)| may overflow a |size_t|.)
-template <typename T>
-bool VerifyUserPointerWithCount(const T* pointer, size_t count) {
-  return internal::VerifyUserPointerWithCountHelper<sizeof(T),
-                                                    MOJO_ALIGNOF(T)>(pointer,
-                                                                     count);
-}
-
-// Verify that |size| bytes (which may be zero) can be read from the user
-// |pointer|, and that |pointer| has the specified |alignment| (if |size| is
-// nonzero).
-template <size_t alignment>
-bool MOJO_SYSTEM_IMPL_EXPORT VerifyUserPointerWithSize(const void* pointer,
-                                                       size_t size);
-
 // Provides a convenient way to implicitly get null |UserPointer<Type>|s.
 struct NullUserPointer {};
 
@@ -214,6 +179,13 @@ class UserPointer {
         static_cast<NonVoidType*>(pointer_) + i));
   }
 
+  // Gets the value of the |UserPointer| as a |uintptr_t|. This should not be
+  // casted back to a pointer (and dereferenced), but may be used as a key for
+  // lookup or passed back to the user.
+  uintptr_t GetPointerValue() const {
+    return reinterpret_cast<uintptr_t>(pointer_);
+  }
+
   // These provides safe (read-only/write-only/read-and-write) access to a
   // |UserPointer<Type>| (probably pointing to an array) using just an ordinary
   // pointer (obtained via |GetPointer()|).
@@ -375,33 +347,6 @@ class UserPointerReaderWriter {
   DISALLOW_COPY_AND_ASSIGN(UserPointerReaderWriter);
 };
 
-// Represents a user pointer that will never be dereferenced by the system. The
-// pointer value (i.e., the address) may be as a key for lookup, or may be a
-// value that is only passed to the user at some point.
-template <typename Type>
-class UserPointerValue {
- public:
-  UserPointerValue() : pointer_() {}
-  explicit UserPointerValue(Type* pointer) : pointer_(pointer) {}
-  ~UserPointerValue() {}
-
-  // Returns the *value* of the pointer, which shouldn't be cast back to a
-  // pointer and dereferenced.
-  uintptr_t GetValue() const {
-    return reinterpret_cast<uintptr_t>(pointer_);
-  }
-
- private:
-  Type* pointer_;
-  // Allow copy and assignment.
-};
-
-// Provides a convenient way to make a |UserPointerValue<Type>|.
-template <typename Type>
-inline UserPointerValue<Type> MakeUserPointerValue(Type* pointer) {
-  return UserPointerValue<Type>(pointer);
-}
-
 }  // namespace system
 }  // namespace mojo
 

mojo/system/memory_unittest.cc

@@ -19,54 +19,150 @@ namespace {
 TEST(MemoryTest, Valid) {
   char my_char;
   int32_t my_int32;
-  int64_t my_int64;
-  char my_char_array[5];
-  int32_t my_int32_array[5];
-  int64_t my_int64_array[5];
-
-  // |VerifyUserPointer|:
-
-  EXPECT_TRUE(VerifyUserPointer<char>(&my_char));
-  EXPECT_TRUE(VerifyUserPointer<int32_t>(&my_int32));
-  EXPECT_TRUE(VerifyUserPointer<int64_t>(&my_int64));
-
-  // |VerifyUserPointerWithCount|:
-
-  EXPECT_TRUE(VerifyUserPointerWithCount<char>(my_char_array, 5));
-  EXPECT_TRUE(VerifyUserPointerWithCount<int32_t>(my_int32_array, 5));
-  EXPECT_TRUE(VerifyUserPointerWithCount<int64_t>(my_int64_array, 5));
-
-  // It shouldn't care what the pointer is if the count is zero.
-  EXPECT_TRUE(VerifyUserPointerWithCount<char>(NULL, 0));
-  EXPECT_TRUE(VerifyUserPointerWithCount<int32_t>(NULL, 0));
-  EXPECT_TRUE(VerifyUserPointerWithCount<int32_t>(
-                  reinterpret_cast<const int32_t*>(1), 0));
-  EXPECT_TRUE(VerifyUserPointerWithCount<int64_t>(NULL, 0));
-  EXPECT_TRUE(VerifyUserPointerWithCount<int64_t>(
-                  reinterpret_cast<const int64_t*>(1), 0));
-
-  // |VerifyUserPointerWithSize|:
-
-  EXPECT_TRUE(VerifyUserPointerWithSize<1>(&my_char, sizeof(my_char)));
-  EXPECT_TRUE(VerifyUserPointerWithSize<1>(&my_int32, sizeof(my_int32)));
-  EXPECT_TRUE(VerifyUserPointerWithSize<MOJO_ALIGNOF(int32_t)>(
-                  &my_int32, sizeof(my_int32)));
-  EXPECT_TRUE(VerifyUserPointerWithSize<1>(&my_int64, sizeof(my_int64)));
-  EXPECT_TRUE(VerifyUserPointerWithSize<MOJO_ALIGNOF(int64_t)>(
-                  &my_int64, sizeof(my_int64)));
-
-  EXPECT_TRUE(VerifyUserPointerWithSize<1>(my_char_array,
-                                           sizeof(my_char_array)));
-  EXPECT_TRUE(VerifyUserPointerWithSize<1>(my_int32_array,
-                                           sizeof(my_int32_array)));
-  EXPECT_TRUE(VerifyUserPointerWithSize<MOJO_ALIGNOF(int32_t)>(
-                  my_int32_array, sizeof(my_int32_array)));
-  EXPECT_TRUE(VerifyUserPointerWithSize<1>(my_int64_array,
-                                           sizeof(my_int64_array)));
-  EXPECT_TRUE(VerifyUserPointerWithSize<MOJO_ALIGNOF(int64_t)>(
-                  my_int64_array, sizeof(my_int64_array)));
+  int64_t my_int64_array[5] = {};  // Zero initialize.
+
+  UserPointer<char> my_char_ptr(&my_char);
+  UserPointer<int32_t> my_int32_ptr(&my_int32);
+  UserPointer<int64_t> my_int64_array_ptr(my_int64_array);
+
+  // |UserPointer<>::IsNull()|:
+  EXPECT_FALSE(my_char_ptr.IsNull());
+  EXPECT_FALSE(my_int32_ptr.IsNull());
+  EXPECT_FALSE(my_int64_array_ptr.IsNull());
+
+  // |UserPointer<>::Put()| and |UserPointer<>::Get()|:
+  my_char_ptr.Put('x');
+  EXPECT_EQ('x', my_char);
+  EXPECT_EQ('x', my_char_ptr.Get());
+  my_int32_ptr.Put(123);
+  EXPECT_EQ(123, my_int32);
+  EXPECT_EQ(123, my_int32_ptr.Get());
+  my_int64_array_ptr.Put(456);
+  EXPECT_EQ(456, my_int64_array[0]);
+  EXPECT_EQ(456, my_int64_array_ptr.Get());
+
+  // |UserPointer<>::At()|, etc.:
+  my_int64_array_ptr.At(3).Put(789);
+  EXPECT_EQ(789, my_int64_array[3]);
+  {
+    // Copy construction:
+    UserPointer<int64_t> other(my_int64_array_ptr.At(3));
+    EXPECT_FALSE(other.IsNull());
+    EXPECT_EQ(789, other.Get());
+
+    // Assignment:
+    other = my_int64_array_ptr;
+    EXPECT_FALSE(other.IsNull());
+    EXPECT_EQ(456, other.Get());
+
+    // Assignment to |NullUserPointer()|:
+    other = NullUserPointer();
+    EXPECT_TRUE(other.IsNull());
+
+    // |MakeUserPointer()|:
+    other = MakeUserPointer(&my_int64_array[1]);
+    other.Put(-123);
+    EXPECT_EQ(-123, my_int64_array_ptr.At(1).Get());
+  }
+
+  // "const" |UserPointer<>|:
+  {
+    // Explicit constructor from |NullUserPointer()|:
+    UserPointer<const char> other((NullUserPointer()));
+    EXPECT_TRUE(other.IsNull());
+
+    // Conversion to "const":
+    other = my_char_ptr;
+    EXPECT_EQ('x', other.Get());
+  }
+
+  // Default constructor:
+  {
+    UserPointer<int32_t> other;
+    EXPECT_TRUE(other.IsNull());
+
+    other = my_int32_ptr;
+    other.Put(-456);
+    EXPECT_EQ(-456, my_int32_ptr.Get());
+  }
+
+  // |UserPointer<>::CheckArray()|:
+  my_int64_array_ptr.CheckArray(5);
+
+  // |UserPointer<>::GetArray()|:
+  {
+    // From a "const" |UserPointer<>| (why not?):
+    UserPointer<const int64_t> other(my_int64_array_ptr);
+    int64_t array[3] = {1, 2, 3};
+    other.At(1).GetArray(array, 3);
+    EXPECT_EQ(-123, array[0]);
+    EXPECT_EQ(0, array[1]);
+    EXPECT_EQ(789, array[2]);
+  }
+
+  // |UserPointer<>::PutArray()|:
+  {
+    const int64_t array[2] = {654, 321};
+    my_int64_array_ptr.At(3).PutArray(array, 2);
+    EXPECT_EQ(0, my_int64_array[2]);
+    EXPECT_EQ(654, my_int64_array[3]);
+    EXPECT_EQ(321, my_int64_array[4]);
+  }
+
+  // |UserPointer<>::Reader|:
+  {
+    UserPointer<int64_t>::Reader reader(my_int64_array_ptr, 5);
+    EXPECT_EQ(456, reader.GetPointer()[0]);
+    EXPECT_EQ(321, reader.GetPointer()[4]);
+  }
+
+  // Non-const to const:
+  {
+    UserPointer<const int64_t>::Reader reader(my_int64_array_ptr.At(3), 1);
+    const int64_t* ptr = reader.GetPointer();
+    EXPECT_EQ(654, *ptr);
+  }
+
+  // |UserPointer<>::Writer|:
+  {
+    UserPointer<int64_t>::Writer writer(my_int64_array_ptr.At(2), 1);
+    int64_t* ptr = writer.GetPointer();
+    *ptr = 1234567890123LL;
+    writer.Commit();
+    EXPECT_EQ(1234567890123LL, my_int64_array[2]);
+  }
+
+  // |UserPointer<>::ReaderWriter|:
+  {
+    UserPointer<int32_t>::ReaderWriter reader_writer(my_int32_ptr, 1);
+    int32_t* ptr = reader_writer.GetPointer();
+    EXPECT_EQ(-456, *ptr);
+    *ptr = 42;
+    reader_writer.Commit();
+    EXPECT_EQ(42, my_int32);
+  }
+
+  // |UserPointer<>::ReinterpretCast<>|:
+  // (This assumes little-endian, etc.)
+  {
+    UserPointer<const char> other(my_int32_ptr.ReinterpretCast<char>());
+    EXPECT_EQ(42, other.Get());
+    EXPECT_EQ(0, other.At(1).Get());
+    EXPECT_EQ(0, other.At(2).Get());
+    EXPECT_EQ(0, other.At(3).Get());
+  }
+
+  // |UserPointer<>::GetPointerValue()|:
+  {
+    UserPointer<int32_t> other;
+    EXPECT_EQ(0u, other.GetPointerValue());
+    other = my_int32_ptr;
+    EXPECT_EQ(reinterpret_cast<uintptr_t>(&my_int32), other.GetPointerValue());
+  }
 }
 
+// TODO(vtl): Convert this test.
+#if 0
 TEST(MemoryTest, Invalid) {
   // Note: |VerifyUserPointer...()| are defined to be "best effort" checks (and
   // may always return true). Thus these tests of invalid cases only reflect the
@@ -130,6 +226,7 @@ TEST(MemoryTest, Invalid) {
   EXPECT_FALSE(VerifyUserPointerWithSize<8>(reinterpret_cast<const int32_t*>(1),
                                             8));
 }
+#endif
 
 }  // namespace
 }  // namespace system