Make SharedMemoryHandle a class on Mac.

base/memory/shared_memory_handle_mac.cc

Index: base/memory/shared_memory_handle_mac.cc
diff --git a/base/memory/shared_memory_handle_mac.cc b/base/memory/shared_memory_handle_mac.cc
new file mode 100644
index 0000000000000000000000000000000000000000..d2ecdb62f61748036dc07a83801b2aad7ea5d39e
--- /dev/null
+++ b/base/memory/shared_memory_handle_mac.cc
@@ -0,0 +1,78 @@
+// Copyright 2015 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 "base/memory/shared_memory_handle.h"
+
+#include <unistd.h>

[Tom Sepez, 2015/06/17 18:51:55]

nit: system includes come first.

[erikchen, 2015/06/18 17:58:33]

In the base/ module (and I think most of chrome), the first include in an
implementation file is the associated header.

For example:
https://code.google.com/p/chromium/codesearch#chromium/src/base/memory/shared...

+
+#include "base/posix/eintr_wrapper.h"
+
+#if defined(OS_MACOSX) && !defined(OS_IOS)
+namespace base {
+
+SharedMemoryHandle::SharedMemoryHandle() : type_(POSIX), file_descriptor_() {
+}
+
+SharedMemoryHandle::SharedMemoryHandle(const FileDescriptor& file_descriptor)
+    : type_(POSIX), file_descriptor_(file_descriptor) {
+}
+
+SharedMemoryHandle::SharedMemoryHandle(int fd, bool auto_close)
+    : type_(POSIX), file_descriptor_(fd, auto_close) {
+}
+
+SharedMemoryHandle::SharedMemoryHandle(const SharedMemoryHandle& handle)
+    : type_(handle.type_), file_descriptor_(handle.file_descriptor_) {
+}
+
+SharedMemoryHandle& SharedMemoryHandle::operator=(
+    const SharedMemoryHandle& handle) {
+  type_ = handle.type_;

[Tom Sepez, 2015/06/17 18:51:55]

Does this make sense?  Do you want to dup the file handle? If I do
  shm2 = shm1;
  close(shm1.fd);
  shm2 becomes invalid despite being separate from shm1.
 
operator= usually requires an if (this != &handle) test.

[erikchen, 2015/06/18 17:58:33]

Added the check to compare "this" and "&handle".

I am intentionally making a shallow copy here. This means that the ownership
semantics of this class are totally messed up. This can't be avoided in the
short term, since base::SharedMemoryHandle is a drop-in replacement for
base::FileDescriptor (on POSIX), which has the exact same, messed up ownership
semantics.

You're right - the example you give would result in an error. Unfortunately, it
is incredibly easy to make that error today, and that can't be easily fixed
without rewriting a lot of code.

+  file_descriptor_ = handle.file_descriptor_;
+  return *this;
+}
+
+bool SharedMemoryHandle::operator==(const SharedMemoryHandle& handle) const {
+  return type_ == handle.type_ && file_descriptor_ == handle.file_descriptor_;
+}
+
+bool SharedMemoryHandle::operator!=(const SharedMemoryHandle& handle) const {
+  return !operator==(handle);
+}

[Tom Sepez, 2015/06/17 18:51:55]

nit: typically avoid invoking operators directly, instead:
  return !(*this == handle);

[erikchen, 2015/06/18 17:58:33]

Done.

+
+SharedMemoryHandle::Type SharedMemoryHandle::GetType() const {
+  return type_;
+}
+
+void SharedMemoryHandle::SetType(SharedMemoryHandle::Type type) {
+  DCHECK(type == POSIX || type == MACH);
+  type_ = type;
+}
+
+void SharedMemoryHandle::SetFileHandle(int fd, bool auto_close) {
+  DCHECK_EQ(type_, POSIX);
+  file_descriptor_.fd = fd;
+  file_descriptor_.auto_close = auto_close;
+}
+
+FileDescriptor* SharedMemoryHandle::GetFileDescriptor() {
+  DCHECK_EQ(type_, POSIX);
+  return &file_descriptor_;
+}
+
+const FileDescriptor* SharedMemoryHandle::GetFileDescriptor() const {
+  DCHECK_EQ(type_, POSIX);
+  return &file_descriptor_;
+}
+
+SharedMemoryHandle SharedMemoryHandle::Duplicate() const {
+  DCHECK_EQ(type_, POSIX);
+  int duped_handle = HANDLE_EINTR(dup(file_descriptor_.fd));
+  if (duped_handle < 0)
+    return SharedMemoryHandle();
+  return SharedMemoryHandle(duped_handle, true);
+}
+
+}  // namespace base
+#endif  // defined(OS_MACOSX) && !defined(OS_IOS)