Add NewMachPort() and its test, and switch call sites to use it.

There were many call sites that wasted a few lines on
mach_port_allocate() and sticking the result into a scoper. I was about
to add three more, so I took the opportunity to simplify things.

TEST=util_test
R=rsesek@chromium.org

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

util/mach/child_port_handshake.cc

@@ -126,11 +126,9 @@ mach_port_t ChildPortHandshake::RunServer() {
 
   // A kqueue cannot monitor a raw Mach receive right with EVFILT_MACHPORT. It
   // requires a port set. Create a new port set and add the receive right to it.
-  mach_port_t server_port_set;
+  base::mac::ScopedMachPortSet server_port_set(
-  kr = mach_port_allocate(
+      NewMachPort(MACH_PORT_RIGHT_PORT_SET));
-      mach_task_self(), MACH_PORT_RIGHT_PORT_SET, &server_port_set);
+  CHECK_NE(server_port_set, kMachPortNull);
-  MACH_CHECK(kr == KERN_SUCCESS, kr) << "mach_port_allocate";
-  base::mac::ScopedMachPortSet server_port_set_owner(server_port_set);
 
   kr = mach_port_insert_member(mach_task_self(), server_port, server_port_set);
   MACH_CHECK(kr == KERN_SUCCESS, kr) << "mach_port_insert_member";

util/mach/mach_extensions.cc

@@ -17,6 +17,7 @@
 #include <AvailabilityMacros.h>
 #include <pthread.h>
 
+#include "base/mac/mach_logging.h"
 #include "util/mac/mac_util.h"
 
 namespace crashpad {
@@ -27,6 +28,13 @@ thread_t MachThreadSelf() {
   return pthread_mach_thread_np(pthread_self());
 }
 
+mach_port_t NewMachPort(mach_port_right_t right) {
+  mach_port_t port = MACH_PORT_NULL;
+  kern_return_t kr = mach_port_allocate(mach_task_self(), right, &port);
+  MACH_LOG_IF(ERROR, kr != KERN_SUCCESS, kr) << "mach_port_allocate";
+  return port;
+}
+
 exception_mask_t ExcMaskAll() {
   // This is necessary because of the way that the kernel validates
   // exception_mask_t arguments to

util/mach/mach_extensions.h

@@ -67,6 +67,17 @@ const exception_type_t kMachExceptionSimulated = 'CPsx';
 //!     thread continues to exist as a `pthread_t`.
 thread_t MachThreadSelf();
 
+//! \brief Creates a new Mach port in the current task.
+//!
+//! This function wraps the `mach_port_allocate()` providing a simpler
+//! interface.
+//!
+//! \param[in] right The type of right to create.
+//!
+//! \return The new Mach port. On failure, `MACH_PORT_NULL` with a message
+//!     logged.
+mach_port_t NewMachPort(mach_port_right_t right);
+
 //! \brief The value for `EXC_MASK_ALL` appropriate for the operating system at
 //!     run time.
 //!

util/mach/mach_extensions_test.cc

@@ -16,6 +16,7 @@
 
 #include "base/mac/scoped_mach_port.h"
 #include "gtest/gtest.h"
+#include "util/test/mac/mach_errors.h"
 
 namespace crashpad {
 namespace test {
@@ -26,6 +27,39 @@ TEST(MachExtensions, MachThreadSelf) {
   EXPECT_EQ(thread_self, MachThreadSelf());
 }
 
+TEST(MachExtensions, NewMachPort_Receive) {
+  base::mac::ScopedMachReceiveRight port(NewMachPort(MACH_PORT_RIGHT_RECEIVE));
+  ASSERT_NE(kMachPortNull, port);
+
+  mach_port_type_t type;
+  kern_return_t kr = mach_port_type(mach_task_self(), port, &type);
+  ASSERT_EQ(KERN_SUCCESS, kr) << MachErrorMessage(kr, "mach_port_get_type");
+
+  EXPECT_EQ(MACH_PORT_TYPE_RECEIVE, type);
+}
+
+TEST(MachExtensions, NewMachPort_PortSet) {
+  base::mac::ScopedMachPortSet port(NewMachPort(MACH_PORT_RIGHT_PORT_SET));
+  ASSERT_NE(kMachPortNull, port);
+
+  mach_port_type_t type;
+  kern_return_t kr = mach_port_type(mach_task_self(), port, &type);
+  ASSERT_EQ(KERN_SUCCESS, kr) << MachErrorMessage(kr, "mach_port_get_type");
+
+  EXPECT_EQ(MACH_PORT_TYPE_PORT_SET, type);
+}
+
+TEST(MachExtensions, NewMachPort_DeadName) {
+  base::mac::ScopedMachSendRight port(NewMachPort(MACH_PORT_RIGHT_DEAD_NAME));
+  ASSERT_NE(kMachPortNull, port);
+
+  mach_port_type_t type;
+  kern_return_t kr = mach_port_type(mach_task_self(), port, &type);
+  ASSERT_EQ(KERN_SUCCESS, kr) << MachErrorMessage(kr, "mach_port_get_type");
+
+  EXPECT_EQ(MACH_PORT_TYPE_DEAD_NAME, type);
+}
+
 }  // namespace
 }  // namespace test
 }  // namespace crashpad

util/mach/mach_message_server_test.cc

@@ -465,12 +465,9 @@ class TestMachMessageServer : public MachMessageServer::Interface,
         // method returns. A send right will be made from this receive right and
         // carried in the request message to the server. By the time the server
         // looks at the right, it will have become a dead name.
-        kr = mach_port_allocate(mach_task_self(),
+        local_receive_port_owner.reset(NewMachPort(MACH_PORT_RIGHT_RECEIVE));
-                                MACH_PORT_RIGHT_RECEIVE,
+        ASSERT_NE(kMachPortNull, local_receive_port_owner);
-                                &request.header.msgh_local_port);
+        request.header.msgh_local_port = local_receive_port_owner;
-        ASSERT_EQ(KERN_SUCCESS, kr)
-            << MachErrorMessage(kr, "mach_port_allocate");
-        local_receive_port_owner.reset(request.header.msgh_local_port);
         break;
       }
     }
@@ -480,12 +477,9 @@ class TestMachMessageServer : public MachMessageServer::Interface,
       // will appear in the parent process. This is used to test that the server
       // properly handles ownership of resources received in complex messages.
       request.body.msgh_descriptor_count = 1;
-      kr = mach_port_allocate(mach_task_self(),
+      child_complex_message_port_.reset(NewMachPort(MACH_PORT_RIGHT_RECEIVE));
-                              MACH_PORT_RIGHT_RECEIVE,
+      ASSERT_NE(kMachPortNull, child_complex_message_port_);
-                              &request.port_descriptor.name);
+      request.port_descriptor.name = child_complex_message_port_;
-      ASSERT_EQ(KERN_SUCCESS, kr)
-          << MachErrorMessage(kr, "mach_port_allocate");
-      child_complex_message_port_.reset(request.port_descriptor.name);
       request.port_descriptor.disposition = MACH_MSG_TYPE_MAKE_SEND;
       request.port_descriptor.type = MACH_MSG_PORT_DESCRIPTOR;
     } else {

util/mach/notify_server_test.cc

@@ -38,21 +38,6 @@ using testing::SetArgPointee;
 using testing::StrictMock;
 using testing::WithArg;
 
-//! \brief Allocates and returns a new receive right.
-//!
-//! \return The new receive right. On failure, `MACH_PORT_NULL` with a gtest
-//!     failure added.
-mach_port_t NewReceiveRight() {
-  mach_port_t receive_right;
-  kern_return_t kr = mach_port_allocate(
-      mach_task_self(), MACH_PORT_RIGHT_RECEIVE, &receive_right);
-  if (kr != KERN_SUCCESS) {
-    EXPECT_EQ(KERN_SUCCESS, kr) << MachErrorMessage(kr, "mach_port_allocate");
-    return MACH_PORT_NULL;
-  }
-  return receive_right;
-}
-
 //! \brief Adds a send right to an existing receive right.
 //!
 //! \param[in] receive_right The receive right to add a send right to.
@@ -272,7 +257,8 @@ class NotifyServerTestBase : public testing::Test,
   //!     with a gtest failure added.
   mach_port_t ServerPort() {
     if (!server_port_) {
-      server_port_.reset(NewReceiveRight());
+      server_port_.reset(NewMachPort(MACH_PORT_RIGHT_RECEIVE));
+      EXPECT_NE(kMachPortNull, server_port_);
     }
 
     return server_port_;
@@ -321,7 +307,8 @@ TEST_F(NotifyServerTest, NoNotification) {
 // When a send-once right with a dead-name notification request is deallocated,
 // a port-deleted notification should be generated.
 TEST_F(NotifyServerTest, MachNotifyPortDeleted) {
-  base::mac::ScopedMachReceiveRight receive_right(NewReceiveRight());
+  base::mac::ScopedMachReceiveRight receive_right(
+      NewMachPort(MACH_PORT_RIGHT_RECEIVE));
   ASSERT_NE(kMachPortNull, receive_right);
 
   base::mac::ScopedMachSendRight send_once_right(
@@ -345,7 +332,8 @@ TEST_F(NotifyServerTest, MachNotifyPortDeleted) {
 // When a receive right with a port-destroyed notification request is destroyed,
 // a port-destroyed notification should be generated.
 TEST_F(NotifyServerTest, MachNotifyPortDestroyed) {
-  base::mac::ScopedMachReceiveRight receive_right(NewReceiveRight());
+  base::mac::ScopedMachReceiveRight receive_right(
+      NewMachPort(MACH_PORT_RIGHT_RECEIVE));
   ASSERT_NE(kMachPortNull, receive_right);
 
   ASSERT_TRUE(RequestMachPortNotification(
@@ -366,7 +354,8 @@ TEST_F(NotifyServerTest, MachNotifyPortDestroyed) {
 // When a receive right with a port-destroyed notification request is not
 // destroyed, no port-destroyed notification should be generated.
 TEST_F(NotifyServerTest, MachNotifyPortDestroyed_NoNotification) {
-  base::mac::ScopedMachReceiveRight receive_right(NewReceiveRight());
+  base::mac::ScopedMachReceiveRight receive_right(
+      NewMachPort(MACH_PORT_RIGHT_RECEIVE));
   ASSERT_NE(kMachPortNull, receive_right);
 
   ASSERT_TRUE(RequestMachPortNotification(
@@ -378,7 +367,8 @@ TEST_F(NotifyServerTest, MachNotifyPortDestroyed_NoNotification) {
 // When a no-senders notification request is registered for a receive right with
 // no senders, a no-senders notification should be generated.
 TEST_F(NotifyServerTest, MachNotifyNoSenders_NoSendRight) {
-  base::mac::ScopedMachReceiveRight receive_right(NewReceiveRight());
+  base::mac::ScopedMachReceiveRight receive_right(
+      NewMachPort(MACH_PORT_RIGHT_RECEIVE));
   ASSERT_NE(kMachPortNull, receive_right);
 
   ASSERT_TRUE(RequestMachPortNotification(
@@ -395,7 +385,8 @@ TEST_F(NotifyServerTest, MachNotifyNoSenders_NoSendRight) {
 // notification request is deallocated, a no-senders notification should be
 // generated.
 TEST_F(NotifyServerTest, MachNotifyNoSenders_SendRightDeallocated) {
-  base::mac::ScopedMachReceiveRight receive_right(NewReceiveRight());
+  base::mac::ScopedMachReceiveRight receive_right(
+      NewMachPort(MACH_PORT_RIGHT_RECEIVE));
   ASSERT_NE(kMachPortNull, receive_right);
 
   base::mac::ScopedMachSendRight send_right(
@@ -417,7 +408,8 @@ TEST_F(NotifyServerTest, MachNotifyNoSenders_SendRightDeallocated) {
 // When the a receive right with a no-senders notification request never loses
 // all senders, no no-senders notification should be generated.
 TEST_F(NotifyServerTest, MachNotifyNoSenders_NoNotification) {
-  base::mac::ScopedMachReceiveRight receive_right(NewReceiveRight());
+  base::mac::ScopedMachReceiveRight receive_right(
+      NewMachPort(MACH_PORT_RIGHT_RECEIVE));
   ASSERT_NE(kMachPortNull, receive_right);
 
   base::mac::ScopedMachSendRight send_right_0(
@@ -459,7 +451,8 @@ TEST_F(NotifyServerTest, MachNotifySendOnce_ExplicitDeallocation) {
 // the send-once right is destroyed, and a send-once notification should appear
 // on the reply port.
 TEST_F(NotifyServerTest, MachNotifySendOnce_ImplicitDeallocation) {
-  base::mac::ScopedMachReceiveRight receive_right(NewReceiveRight());
+  base::mac::ScopedMachReceiveRight receive_right(
+      NewMachPort(MACH_PORT_RIGHT_RECEIVE));
   ASSERT_NE(kMachPortNull, receive_right);
 
   mach_msg_empty_send_t message = {};
@@ -490,7 +483,8 @@ TEST_F(NotifyServerTest, MachNotifySendOnce_ImplicitDeallocation) {
 // notification request is destroyed, a dead-name notification should be
 // generated.
 TEST_F(NotifyServerTest, MachNotifyDeadName) {
-  base::mac::ScopedMachReceiveRight receive_right(NewReceiveRight());
+  base::mac::ScopedMachReceiveRight receive_right(
+      NewMachPort(MACH_PORT_RIGHT_RECEIVE));
   ASSERT_NE(kMachPortNull, receive_right);
 
   base::mac::ScopedMachSendRight send_once_right(
@@ -527,7 +521,8 @@ TEST_F(NotifyServerTest, MachNotifyDeadName) {
 // notification request is not destroyed, no dead-name notification should be
 // generated.
 TEST_F(NotifyServerTest, MachNotifyDeadName_NoNotification) {
-  base::mac::ScopedMachReceiveRight receive_right(NewReceiveRight());
+  base::mac::ScopedMachReceiveRight receive_right(
+      NewMachPort(MACH_PORT_RIGHT_RECEIVE));
   ASSERT_NE(kMachPortNull, receive_right);
 
   base::mac::ScopedMachSendRight send_once_right(

util/test/mac/mach_multiprocess.cc

@@ -215,15 +215,12 @@ void MachMultiprocess::MultiprocessChild() {
   // local_port is not valid in the forked child process.
   ignore_result(info_->local_port.release());
 
-  mach_port_t local_port;
+  info_->local_port.reset(NewMachPort(MACH_PORT_RIGHT_RECEIVE));
-  kern_return_t kr = mach_port_allocate(
+  ASSERT_NE(kMachPortNull, info_->local_port);
-      mach_task_self(), MACH_PORT_RIGHT_RECEIVE, &local_port);
-  ASSERT_EQ(KERN_SUCCESS, kr) << MachErrorMessage(kr, "mach_port_allocate");
-  info_->local_port.reset(local_port);
 
   // The remote port can be obtained from the bootstrap server.
   mach_port_t remote_port;
-  kr = bootstrap_look_up(
+  kern_return_t kr = bootstrap_look_up(
       bootstrap_port, info_->service_name.c_str(), &remote_port);
   ASSERT_EQ(BOOTSTRAP_SUCCESS, kr)
       << BootstrapErrorMessage(kr, "bootstrap_look_up");