pdx: Rework error reporting when transfering file and channel handles

There is a lot of confusion about reporting errors when passing file
and channel handles over PDX transport between client and service.

Methods like Message::PushFileHandle return an integer which means
both a file handle reference value (if positive) and a possible error
code (if negative). But file handles could contain negative values too
(when they are empty). This is used frequently when passing buffer
fences around (when a fence is not being used, its fd is set to -1).

This results in a special case of when PushFileHandle is called with
a file handle with value of -1, the return value is actually "-errno"
which becomes dependent on a global state (errno is not set by
PushFileHandle itself in case file handle value is negative) and results
in unpredicted behavior (sometimes errno is 0, sometimes its >0).

Cleaned this all up by using Status<T> everywhere we used an int to
pass value payload along with possible error code.

Now the semantics of the calls are more clear.

Bug: 36866492
Test: `m -j32` for sailfish-eng succeeds
      Ran unit tests on device (pdx_tests, libpdx_uds_tests, bufferhub_tests,
      buffer_hub_queue-test, buffer_hub_queue_producer-test), all pass
      Ran CubeSea, NativeTreasureHunt and Ithaca on Sailfish with vrflinger
      enabled, was able to use controller and screen rendered correctly.

Change-Id: I0f40c3f356fcba8bc217d5219a0ddf9685e57fd7
diff --git a/services/vr/bufferhubd/buffer_hub.cpp b/services/vr/bufferhubd/buffer_hub.cpp
index 80efcf8..de4950e 100644
--- a/services/vr/bufferhubd/buffer_hub.cpp
+++ b/services/vr/bufferhubd/buffer_hub.cpp
@@ -189,7 +189,7 @@
     channel->HandleImpulse(message);
 }
 
-int BufferHubService::HandleMessage(Message& message) {
+pdx::Status<void> BufferHubService::HandleMessage(Message& message) {
   ATRACE_NAME("BufferHubService::HandleMessage");
   auto channel = message.GetChannel<BufferHubChannel>();
 
@@ -207,22 +207,22 @@
     case BufferHubRPC::CreateBuffer::Opcode:
       DispatchRemoteMethod<BufferHubRPC::CreateBuffer>(
           *this, &BufferHubService::OnCreateBuffer, message);
-      return 0;
+      return {};
 
     case BufferHubRPC::CreatePersistentBuffer::Opcode:
       DispatchRemoteMethod<BufferHubRPC::CreatePersistentBuffer>(
           *this, &BufferHubService::OnCreatePersistentBuffer, message);
-      return 0;
+      return {};
 
     case BufferHubRPC::GetPersistentBuffer::Opcode:
       DispatchRemoteMethod<BufferHubRPC::GetPersistentBuffer>(
           *this, &BufferHubService::OnGetPersistentBuffer, message);
-      return 0;
+      return {};
 
     case BufferHubRPC::CreateProducerQueue::Opcode:
       DispatchRemoteMethod<BufferHubRPC::CreateProducerQueue>(
           *this, &BufferHubService::OnCreateProducerQueue, message);
-      return 0;
+      return {};
 
     default:
       return DefaultHandleMessage(message);
@@ -438,11 +438,11 @@
            "BufferHubChannel::SignalAvailable: channel_id=%d buffer_id=%d",
            channel_id(), buffer_id());
   if (!IsDetached()) {
-    const int ret = service_->ModifyChannelEvents(channel_id_, 0, POLLIN);
-    ALOGE_IF(ret < 0,
+    const auto status = service_->ModifyChannelEvents(channel_id_, 0, POLLIN);
+    ALOGE_IF(!status,
              "BufferHubChannel::SignalAvailable: failed to signal availability "
              "channel_id=%d: %s",
-             channel_id_, strerror(-ret));
+             channel_id_, status.GetErrorMessage().c_str());
   } else {
     ALOGD_IF(TRACE, "BufferHubChannel::SignalAvailable: detached buffer.");
   }
@@ -454,11 +454,11 @@
            "BufferHubChannel::ClearAvailable: channel_id=%d buffer_id=%d",
            channel_id(), buffer_id());
   if (!IsDetached()) {
-    const int ret = service_->ModifyChannelEvents(channel_id_, POLLIN, 0);
-    ALOGE_IF(ret < 0,
+    const auto status = service_->ModifyChannelEvents(channel_id_, POLLIN, 0);
+    ALOGE_IF(!status,
              "BufferHubChannel::ClearAvailable: failed to clear availability "
              "channel_id=%d: %s",
-             channel_id_, strerror(-ret));
+             channel_id_, status.GetErrorMessage().c_str());
   } else {
     ALOGD_IF(TRACE, "BufferHubChannel::ClearAvailable: detached buffer.");
   }
@@ -469,11 +469,11 @@
   ALOGD_IF(TRACE, "BufferHubChannel::Hangup: channel_id=%d buffer_id=%d",
            channel_id(), buffer_id());
   if (!IsDetached()) {
-    const int ret = service_->ModifyChannelEvents(channel_id_, 0, POLLHUP);
+    const auto status = service_->ModifyChannelEvents(channel_id_, 0, POLLHUP);
     ALOGE_IF(
-        ret < 0,
+        !status,
         "BufferHubChannel::Hangup: failed to signal hangup channel_id=%d: %s",
-        channel_id_, strerror(-ret));
+        channel_id_, status.GetErrorMessage().c_str());
   } else {
     ALOGD_IF(TRACE, "BufferHubChannel::Hangup: detached buffer.");
   }
diff --git a/services/vr/bufferhubd/buffer_hub.h b/services/vr/bufferhubd/buffer_hub.h
index 28cb468..8a7dca8 100644
--- a/services/vr/bufferhubd/buffer_hub.h
+++ b/services/vr/bufferhubd/buffer_hub.h
@@ -141,7 +141,7 @@
   BufferHubService();
   ~BufferHubService() override;
 
-  int HandleMessage(pdx::Message& message) override;
+  pdx::Status<void> HandleMessage(pdx::Message& message) override;
   void HandleImpulse(pdx::Message& message) override;
 
   void OnChannelClose(pdx::Message& message,
diff --git a/services/vr/bufferhubd/producer_channel.cpp b/services/vr/bufferhubd/producer_channel.cpp
index 43010b0..903d174 100644
--- a/services/vr/bufferhubd/producer_channel.cpp
+++ b/services/vr/bufferhubd/producer_channel.cpp
@@ -161,12 +161,12 @@
 
   auto consumer = std::make_shared<ConsumerChannel>(
       service(), buffer_id(), channel_id, shared_from_this());
-  const int ret = service()->SetChannel(channel_id, consumer);
-  if (ret < 0) {
+  const auto channel_status = service()->SetChannel(channel_id, consumer);
+  if (!channel_status) {
     ALOGE(
         "ProducerChannel::CreateConsumer: failed to set new consumer channel: "
         "%s",
-        strerror(-ret));
+        channel_status.GetErrorMessage().c_str());
     return RemoteChannelHandle();
   }
 
diff --git a/services/vr/bufferhubd/producer_queue_channel.cpp b/services/vr/bufferhubd/producer_queue_channel.cpp
index 08f1e9d..7952642 100644
--- a/services/vr/bufferhubd/producer_queue_channel.cpp
+++ b/services/vr/bufferhubd/producer_queue_channel.cpp
@@ -97,18 +97,18 @@
         "ProducerQueueChannel::OnCreateConsumerQueue: failed to push consumer "
         "channel: %s",
         status.GetErrorMessage().c_str());
-    REPLY_ERROR_RETURN(message, ENOMEM, {});
+    REPLY_ERROR_RETURN(message, status.error(), {});
   }
 
-  const int ret = service()->SetChannel(
+  const auto channel_status = service()->SetChannel(
       channel_id, std::make_shared<ConsumerQueueChannel>(
                       service(), buffer_id(), channel_id, shared_from_this()));
-  if (ret < 0) {
+  if (!channel_status) {
     ALOGE(
         "ProducerQueueChannel::OnCreateConsumerQueue: failed to set new "
         "consumer channel: %s",
-        strerror(-ret));
-    REPLY_ERROR_RETURN(message, ENOMEM, {});
+        channel_status.GetErrorMessage().c_str());
+    REPLY_ERROR_RETURN(message, channel_status.error(), {});
   }
 
   return std::make_pair(status.take(), meta_size_bytes_);
@@ -214,12 +214,13 @@
       "ProducerQueueChannel::AllocateBuffer: buffer_id=%d, buffer_handle=%d",
       buffer_id, buffer_handle.value());
 
-  const int ret = service()->SetChannel(buffer_id, producer_channel);
-  if (ret < 0) {
+  const auto channel_status =
+      service()->SetChannel(buffer_id, producer_channel);
+  if (!channel_status) {
     ALOGE(
-        "ProducerQueueChannel::AllocateBuffer: failed to set prodcuer channel "
+        "ProducerQueueChannel::AllocateBuffer: failed to set producer channel "
         "for new BufferHubBuffer: %s",
-        strerror(-ret));
+        channel_status.GetErrorMessage().c_str());
     return {};
   }
 
@@ -252,7 +253,7 @@
   return {std::move(buffer_handle), slot};
 }
 
-int ProducerQueueChannel::OnProducerQueueDetachBuffer(Message& message,
+int ProducerQueueChannel::OnProducerQueueDetachBuffer(Message& /*message*/,
                                                       size_t slot) {
   if (buffers_[slot].expired()) {
     ALOGE(
diff --git a/services/vr/performanced/performance_service.cpp b/services/vr/performanced/performance_service.cpp
index c99c8d4..955c661 100644
--- a/services/vr/performanced/performance_service.cpp
+++ b/services/vr/performanced/performance_service.cpp
@@ -170,22 +170,22 @@
   return task.GetCpuSetPath();
 }
 
-int PerformanceService::HandleMessage(Message& message) {
+pdx::Status<void> PerformanceService::HandleMessage(Message& message) {
   switch (message.GetOp()) {
     case PerformanceRPC::SetCpuPartition::Opcode:
       DispatchRemoteMethod<PerformanceRPC::SetSchedulerClass>(
           *this, &PerformanceService::OnSetCpuPartition, message);
-      return 0;
+      return {};
 
     case PerformanceRPC::SetSchedulerClass::Opcode:
       DispatchRemoteMethod<PerformanceRPC::SetSchedulerClass>(
           *this, &PerformanceService::OnSetSchedulerClass, message);
-      return 0;
+      return {};
 
     case PerformanceRPC::GetCpuPartition::Opcode:
       DispatchRemoteMethod<PerformanceRPC::GetCpuPartition>(
           *this, &PerformanceService::OnGetCpuPartition, message);
-      return 0;
+      return {};
 
     default:
       return Service::HandleMessage(message);
diff --git a/services/vr/performanced/performance_service.h b/services/vr/performanced/performance_service.h
index e32d834..34abba7 100644
--- a/services/vr/performanced/performance_service.h
+++ b/services/vr/performanced/performance_service.h
@@ -17,7 +17,7 @@
 // achieve system performance goals.
 class PerformanceService : public pdx::ServiceBase<PerformanceService> {
  public:
-  int HandleMessage(pdx::Message& message) override;
+  pdx::Status<void> HandleMessage(pdx::Message& message) override;
   bool IsInitialized() const override;
 
   std::string DumpState(size_t max_length) override;
diff --git a/services/vr/sensord/pose_service.cpp b/services/vr/sensord/pose_service.cpp
index 2c4fc30..3cd5297 100644
--- a/services/vr/sensord/pose_service.cpp
+++ b/services/vr/sensord/pose_service.cpp
@@ -476,8 +476,8 @@
   }
 }
 
-int PoseService::HandleMessage(pdx::Message& msg) {
-  int ret = 0;
+pdx::Status<void> PoseService::HandleMessage(pdx::Message& msg) {
+  pdx::Status<void> ret;
   const pdx::MessageInfo& info = msg.GetInfo();
   switch (info.op) {
     case DVR_POSE_NOTIFY_VSYNC: {
@@ -495,21 +495,13 @@
           {.iov_base = &right_eye_photon_offset_ns_,
            .iov_len = sizeof(right_eye_photon_offset_ns_)},
       };
-      constexpr int expected_size =
-          sizeof(vsync_count_) + sizeof(photon_timestamp_) +
-          sizeof(display_period_ns_) + sizeof(right_eye_photon_offset_ns_);
-      ret = msg.ReadVector(data, sizeof(data) / sizeof(data[0]));
-      if (ret < expected_size) {
-        ALOGI("error: msg.Read read too little (%d < %d)", ret, expected_size);
-        REPLY_ERROR(msg, EIO, error);
-      }
-
-      if (!enable_external_pose_) {
+      ret = msg.ReadVectorAll(data);
+      if (ret && !enable_external_pose_) {
         mapped_pose_buffer_->vsync_count = vsync_count_;
       }
 
       // TODO(jbates, eieio): make this async, no need to reply.
-      REPLY_SUCCESS(msg, 0, error);
+      REPLY_MESSAGE(msg, ret, error);
     }
     case DVR_POSE_POLL: {
       ATRACE_NAME("pose_poll");
@@ -535,61 +527,43 @@
 
       Btrace("Pose polled");
 
-      ret = msg.Write(&client_state, sizeof(client_state));
-      const int expected_size = sizeof(client_state);
-      if (ret < expected_size) {
-        ALOGI("error: msg.Write wrote too little (%d < %d)", ret,
-              expected_size);
-        REPLY_ERROR(msg, EIO, error);
-      }
-      REPLY_SUCCESS(msg, 0, error);
+      ret = msg.WriteAll(&client_state, sizeof(client_state));
+      REPLY_MESSAGE(msg, ret, error);
     }
     case DVR_POSE_FREEZE: {
       {
         std::lock_guard<std::mutex> guard(mutex_);
 
         DvrPoseState frozen_state;
-        const int expected_size = sizeof(frozen_state);
-        ret = msg.Read(&frozen_state, expected_size);
-        if (ret < expected_size) {
-          ALOGI("error: msg.Read read too little (%d < %d)", ret,
-                expected_size);
-          REPLY_ERROR(msg, EIO, error);
+        ret = msg.ReadAll(&frozen_state, sizeof(frozen_state));
+        if (!ret) {
+          REPLY_ERROR(msg, ret.error(), error);
         }
         frozen_state_ = frozen_state;
       }
       SetPoseMode(DVR_POSE_MODE_MOCK_FROZEN);
-      REPLY_SUCCESS(msg, 0, error);
+      REPLY_MESSAGE(msg, ret, error);
     }
     case DVR_POSE_SET_MODE: {
       int mode;
       {
         std::lock_guard<std::mutex> guard(mutex_);
-        const int expected_size = sizeof(mode);
-        ret = msg.Read(&mode, expected_size);
-        if (ret < expected_size) {
-          ALOGI("error: msg.Read read too little (%d < %d)", ret,
-                expected_size);
-          REPLY_ERROR(msg, EIO, error);
+        ret = msg.ReadAll(&mode, sizeof(mode));
+        if (!ret) {
+          REPLY_ERROR(msg, ret.error(), error);
         }
         if (mode < 0 || mode >= DVR_POSE_MODE_COUNT) {
           REPLY_ERROR(msg, EINVAL, error);
         }
       }
       SetPoseMode(DvrPoseMode(mode));
-      REPLY_SUCCESS(msg, 0, error);
+      REPLY_MESSAGE(msg, ret, error);
     }
     case DVR_POSE_GET_MODE: {
       std::lock_guard<std::mutex> guard(mutex_);
       int mode = pose_mode_;
-      ret = msg.Write(&mode, sizeof(mode));
-      const int expected_size = sizeof(mode);
-      if (ret < expected_size) {
-        ALOGI("error: msg.Write wrote too little (%d < %d)", ret,
-              expected_size);
-        REPLY_ERROR(msg, EIO, error);
-      }
-      REPLY_SUCCESS(msg, 0, error);
+      ret = msg.WriteAll(&mode, sizeof(mode));
+      REPLY_MESSAGE(msg, ret, error);
     }
     case DVR_POSE_GET_RING_BUFFER: {
       std::lock_guard<std::mutex> guard(mutex_);
diff --git a/services/vr/sensord/pose_service.h b/services/vr/sensord/pose_service.h
index 4df5036..fdd29b5 100644
--- a/services/vr/sensord/pose_service.h
+++ b/services/vr/sensord/pose_service.h
@@ -27,7 +27,7 @@
   ~PoseService() override;
 
   bool IsInitialized() const override;
-  int HandleMessage(pdx::Message& msg) override;
+  pdx::Status<void> HandleMessage(pdx::Message& msg) override;
   std::string DumpState(size_t max_length) override;
 
   // Handle events from the sensor HAL.
diff --git a/services/vr/sensord/sensor_service.cpp b/services/vr/sensord/sensor_service.cpp
index 1b809b0..a182a26 100644
--- a/services/vr/sensord/sensor_service.cpp
+++ b/services/vr/sensord/sensor_service.cpp
@@ -69,8 +69,8 @@
   client->unset_sensor();
 }
 
-int SensorService::HandleMessage(pdx::Message& msg) {
-  int ret = 0;
+pdx::Status<void> SensorService::HandleMessage(pdx::Message& msg) {
+  pdx::Status<void> ret;
   const pdx::MessageInfo& info = msg.GetInfo();
   switch (info.op) {
     case DVR_SENSOR_START: {
@@ -82,8 +82,7 @@
       if (client->has_sensor())
         REPLY_ERROR(msg, EINVAL, error);
       int sensor_type;
-      if (msg.Read(&sensor_type, sizeof(sensor_type)) <
-          (ssize_t)sizeof(sensor_type))
+      if (!msg.ReadAll(&sensor_type, sizeof(sensor_type)))
         REPLY_ERROR(msg, EIO, error);
 
       // Find the sensor of the requested type.
@@ -120,10 +119,8 @@
           {.iov_base = out_buffer,
            .iov_len = num_events * sizeof(sensors_event_t)},
       };
-      ret = msg.WriteVector(svec, 2);
-      int expected_size = sizeof(int) + num_events * sizeof(sensors_event_t);
-      if (ret < expected_size) {
-        ALOGI("error: msg.WriteVector wrote too little.");
+      ret = msg.WriteVectorAll(svec, 2);
+      if (!ret) {
         REPLY_ERROR(msg, EIO, error);
       }
       REPLY_SUCCESS(msg, 0, error);
diff --git a/services/vr/sensord/sensor_service.h b/services/vr/sensord/sensor_service.h
index c35fada..6ea470b 100644
--- a/services/vr/sensord/sensor_service.h
+++ b/services/vr/sensord/sensor_service.h
@@ -22,7 +22,7 @@
  */
 class SensorService : public pdx::ServiceBase<SensorService> {
  public:
-  int HandleMessage(pdx::Message& msg) override;
+  pdx::Status<void> HandleMessage(pdx::Message& msg) override;
   std::shared_ptr<pdx::Channel> OnChannelOpen(pdx::Message& msg) override;
   void OnChannelClose(pdx::Message& msg,
                       const std::shared_ptr<pdx::Channel>& chan) override;