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/libs/vr/libvrflinger/display_surface.cpp b/libs/vr/libvrflinger/display_surface.cpp
index 66e9925..a7220fe 100644
--- a/libs/vr/libvrflinger/display_surface.cpp
+++ b/libs/vr/libvrflinger/display_surface.cpp
@@ -206,7 +206,7 @@
return !acquired_buffers_.IsEmpty();
}
-int DisplaySurface::HandleMessage(pdx::Message& message) {
+pdx::Status<void> DisplaySurface::HandleMessage(pdx::Message& message) {
switch (message.GetOp()) {
case DisplayRPC::SetAttributes::Opcode:
DispatchRemoteMethod<DisplayRPC::SetAttributes>(
@@ -227,7 +227,7 @@
return SurfaceChannel::HandleMessage(message);
}
- return 0;
+ return {};
}
int DisplaySurface::OnClientSetAttributes(
@@ -301,7 +301,7 @@
pdx::Message& message) {
if (flags_ & DVR_DISPLAY_SURFACE_FLAGS_DISABLE_SYSTEM_DISTORTION) {
ALOGE(
- "DisplaySurface::OnCreateVideoMeshSurface: system distorion is "
+ "DisplaySurface::OnCreateVideoMeshSurface: system distortion is "
"disabled on this display surface, cannot create VideoMeshSurface on "
"top of it.");
REPLY_ERROR_RETURN(message, EINVAL, {});
@@ -309,22 +309,21 @@
int channel_id;
auto status = message.PushChannel(0, nullptr, &channel_id);
-
if (!status) {
ALOGE(
"DisplaySurface::OnCreateVideoMeshSurface: failed to push channel: %s",
status.GetErrorMessage().c_str());
- REPLY_ERROR_RETURN(message, ENOMEM, {});
+ REPLY_ERROR_RETURN(message, status.error(), {});
}
auto surface = std::make_shared<VideoMeshSurface>(service(), channel_id);
- const int ret = service()->SetChannel(channel_id, surface);
- if (ret < 0) {
+ auto channel_status = service()->SetChannel(channel_id, surface);
+ if (!channel_status) {
ALOGE(
"DisplaySurface::OnCreateVideoMeshSurface: failed to set new video "
"mesh surface channel: %s",
- strerror(-ret));
- REPLY_ERROR_RETURN(message, ENOMEM, {});
+ channel_status.GetErrorMessage().c_str());
+ REPLY_ERROR_RETURN(message, channel_status.error(), {});
}
{