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.");
}