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/libpdx_uds/Android.bp b/libs/vr/libpdx_uds/Android.bp
index 09eeaa0..9f308ec 100644
--- a/libs/vr/libpdx_uds/Android.bp
+++ b/libs/vr/libpdx_uds/Android.bp
@@ -41,6 +41,8 @@
"libpdx",
],
shared_libs: [
+ "libbase",
+ "libcutils",
"liblog",
"libutils",
],
diff --git a/libs/vr/libpdx_uds/ipc_helper.cpp b/libs/vr/libpdx_uds/ipc_helper.cpp
index ee7299e..fe5c986 100644
--- a/libs/vr/libpdx_uds/ipc_helper.cpp
+++ b/libs/vr/libpdx_uds/ipc_helper.cpp
@@ -87,7 +87,7 @@
OutputResourceMapper* SendPayload::GetOutputResourceMapper() { return this; }
// OutputResourceMapper
-FileReference SendPayload::PushFileHandle(const LocalHandle& handle) {
+Status<FileReference> SendPayload::PushFileHandle(const LocalHandle& handle) {
if (handle) {
const int ref = file_handles_.size();
file_handles_.push_back(handle.Get());
@@ -97,7 +97,8 @@
}
}
-FileReference SendPayload::PushFileHandle(const BorrowedHandle& handle) {
+Status<FileReference> SendPayload::PushFileHandle(
+ const BorrowedHandle& handle) {
if (handle) {
const int ref = file_handles_.size();
file_handles_.push_back(handle.Get());
@@ -107,21 +108,21 @@
}
}
-FileReference SendPayload::PushFileHandle(const RemoteHandle& handle) {
+Status<FileReference> SendPayload::PushFileHandle(const RemoteHandle& handle) {
return handle.Get();
}
-ChannelReference SendPayload::PushChannelHandle(
+Status<ChannelReference> SendPayload::PushChannelHandle(
const LocalChannelHandle& /*handle*/) {
- return -1;
+ return ErrorStatus{EOPNOTSUPP};
}
-ChannelReference SendPayload::PushChannelHandle(
+Status<ChannelReference> SendPayload::PushChannelHandle(
const BorrowedChannelHandle& /*handle*/) {
- return -1;
+ return ErrorStatus{EOPNOTSUPP};
}
-ChannelReference SendPayload::PushChannelHandle(
+Status<ChannelReference> SendPayload::PushChannelHandle(
const RemoteChannelHandle& /*handle*/) {
- return -1;
+ return ErrorStatus{EOPNOTSUPP};
}
Status<void> ReceivePayload::Receive(int socket_fd) {
diff --git a/libs/vr/libpdx_uds/private/uds/ipc_helper.h b/libs/vr/libpdx_uds/private/uds/ipc_helper.h
index 00f3490..80530bf 100644
--- a/libs/vr/libpdx_uds/private/uds/ipc_helper.h
+++ b/libs/vr/libpdx_uds/private/uds/ipc_helper.h
@@ -33,13 +33,14 @@
OutputResourceMapper* GetOutputResourceMapper() override;
// OutputResourceMapper
- FileReference PushFileHandle(const LocalHandle& handle) override;
- FileReference PushFileHandle(const BorrowedHandle& handle) override;
- FileReference PushFileHandle(const RemoteHandle& handle) override;
- ChannelReference PushChannelHandle(const LocalChannelHandle& handle) override;
- ChannelReference PushChannelHandle(
+ Status<FileReference> PushFileHandle(const LocalHandle& handle) override;
+ Status<FileReference> PushFileHandle(const BorrowedHandle& handle) override;
+ Status<FileReference> PushFileHandle(const RemoteHandle& handle) override;
+ Status<ChannelReference> PushChannelHandle(
+ const LocalChannelHandle& handle) override;
+ Status<ChannelReference> PushChannelHandle(
const BorrowedChannelHandle& handle) override;
- ChannelReference PushChannelHandle(
+ Status<ChannelReference> PushChannelHandle(
const RemoteChannelHandle& handle) override;
private:
diff --git a/libs/vr/libpdx_uds/private/uds/service_endpoint.h b/libs/vr/libpdx_uds/private/uds/service_endpoint.h
index 3ec8519..9d038cb 100644
--- a/libs/vr/libpdx_uds/private/uds/service_endpoint.h
+++ b/libs/vr/libpdx_uds/private/uds/service_endpoint.h
@@ -39,41 +39,40 @@
~Endpoint() override = default;
uint32_t GetIpcTag() const override { return kIpcTag; }
- int SetService(Service* service) override;
- int SetChannel(int channel_id, Channel* channel) override;
- int CloseChannel(int channel_id) override;
- int ModifyChannelEvents(int channel_id, int clear_mask,
- int set_mask) override;
+ Status<void> SetService(Service* service) override;
+ Status<void> SetChannel(int channel_id, Channel* channel) override;
+ Status<void> CloseChannel(int channel_id) override;
+ Status<void> ModifyChannelEvents(int channel_id, int clear_mask,
+ int set_mask) override;
Status<RemoteChannelHandle> PushChannel(Message* message, int flags,
Channel* channel,
int* channel_id) override;
Status<int> CheckChannel(const Message* message, ChannelReference ref,
Channel** channel) override;
- int DefaultHandleMessage(const MessageInfo& info) override;
- int MessageReceive(Message* message) override;
- int MessageReply(Message* message, int return_code) override;
- int MessageReplyFd(Message* message, unsigned int push_fd) override;
- int MessageReplyChannelHandle(Message* message,
- const LocalChannelHandle& handle) override;
- int MessageReplyChannelHandle(Message* message,
- const BorrowedChannelHandle& handle) override;
- int MessageReplyChannelHandle(Message* message,
- const RemoteChannelHandle& handle) override;
- ssize_t ReadMessageData(Message* message, const iovec* vector,
- size_t vector_length) override;
- ssize_t WriteMessageData(Message* message, const iovec* vector,
- size_t vector_length) override;
- FileReference PushFileHandle(Message* message,
- const LocalHandle& handle) override;
- FileReference PushFileHandle(Message* message,
- const BorrowedHandle& handle) override;
- FileReference PushFileHandle(Message* message,
- const RemoteHandle& handle) override;
- ChannelReference PushChannelHandle(Message* message,
- const LocalChannelHandle& handle) override;
- ChannelReference PushChannelHandle(
+ Status<void> MessageReceive(Message* message) override;
+ Status<void> MessageReply(Message* message, int return_code) override;
+ Status<void> MessageReplyFd(Message* message, unsigned int push_fd) override;
+ Status<void> MessageReplyChannelHandle(
+ Message* message, const LocalChannelHandle& handle) override;
+ Status<void> MessageReplyChannelHandle(
Message* message, const BorrowedChannelHandle& handle) override;
- ChannelReference PushChannelHandle(
+ Status<void> MessageReplyChannelHandle(
+ Message* message, const RemoteChannelHandle& handle) override;
+ Status<size_t> ReadMessageData(Message* message, const iovec* vector,
+ size_t vector_length) override;
+ Status<size_t> WriteMessageData(Message* message, const iovec* vector,
+ size_t vector_length) override;
+ Status<FileReference> PushFileHandle(Message* message,
+ const LocalHandle& handle) override;
+ Status<FileReference> PushFileHandle(Message* message,
+ const BorrowedHandle& handle) override;
+ Status<FileReference> PushFileHandle(Message* message,
+ const RemoteHandle& handle) override;
+ Status<ChannelReference> PushChannelHandle(
+ Message* message, const LocalChannelHandle& handle) override;
+ Status<ChannelReference> PushChannelHandle(
+ Message* message, const BorrowedChannelHandle& handle) override;
+ Status<ChannelReference> PushChannelHandle(
Message* message, const RemoteChannelHandle& handle) override;
LocalHandle GetFileHandle(Message* message, FileReference ref) const override;
LocalChannelHandle GetChannelHandle(Message* message,
@@ -82,15 +81,22 @@
void* AllocateMessageState() override;
void FreeMessageState(void* state) override;
- int Cancel() override;
+ Status<void> Cancel() override;
// Open an endpoint at the given path.
// Second parameter is unused for UDS, but we have it here for compatibility
// in signature with servicefs::Endpoint::Create().
+ // This method uses |endpoint_path| as a relative path to endpoint socket
+ // created by init process.
static std::unique_ptr<Endpoint> Create(const std::string& endpoint_path,
mode_t /*unused_mode*/ = kDefaultMode,
bool blocking = kDefaultBlocking);
+ // Helper method to create an endpoint at the given UDS socket path. This
+ // method physically creates and binds a socket at that path.
+ static std::unique_ptr<Endpoint> CreateAndBindSocket(
+ const std::string& endpoint_path, bool blocking = kDefaultBlocking);
+
int epoll_fd() const { return epoll_fd_.Get(); }
private:
@@ -117,7 +123,7 @@
Status<void> OnNewChannel(LocalHandle channel_fd);
Status<ChannelData*> OnNewChannelLocked(LocalHandle channel_fd,
Channel* channel_state);
- int CloseChannelLocked(int channel_id);
+ Status<void> CloseChannelLocked(int channel_id);
Status<void> ReenableEpollEvent(int fd);
Channel* GetChannelState(int channel_id);
int GetChannelSocketFd(int channel_id);
diff --git a/libs/vr/libpdx_uds/remote_method_tests.cpp b/libs/vr/libpdx_uds/remote_method_tests.cpp
index 9050500..3109753 100644
--- a/libs/vr/libpdx_uds/remote_method_tests.cpp
+++ b/libs/vr/libpdx_uds/remote_method_tests.cpp
@@ -342,87 +342,87 @@
// Test service that encodes/decodes messages from clients.
class TestService : public ServiceBase<TestService> {
public:
- int HandleMessage(Message& message) override {
+ Status<void> HandleMessage(Message& message) override {
switch (message.GetOp()) {
case TestInterface::Add::Opcode:
DispatchRemoteMethod<TestInterface::Add>(*this, &TestService::OnAdd,
message);
- return 0;
+ return {};
case TestInterface::Foo::Opcode:
DispatchRemoteMethod<TestInterface::Foo>(*this, &TestService::OnFoo,
message);
- return 0;
+ return {};
case TestInterface::Concatenate::Opcode:
DispatchRemoteMethod<TestInterface::Concatenate>(
*this, &TestService::OnConcatenate, message);
- return 0;
+ return {};
case TestInterface::SumVector::Opcode:
DispatchRemoteMethod<TestInterface::SumVector>(
*this, &TestService::OnSumVector, message);
- return 0;
+ return {};
case TestInterface::StringLength::Opcode:
DispatchRemoteMethod<TestInterface::StringLength>(
*this, &TestService::OnStringLength, message);
- return 0;
+ return {};
case TestInterface::SendTestType::Opcode:
DispatchRemoteMethod<TestInterface::SendTestType>(
*this, &TestService::OnSendTestType, message);
- return 0;
+ return {};
case TestInterface::SendVector::Opcode:
DispatchRemoteMethod<TestInterface::SendVector>(
*this, &TestService::OnSendVector, message);
- return 0;
+ return {};
case TestInterface::Rot13::Opcode:
DispatchRemoteMethod<TestInterface::Rot13>(*this, &TestService::OnRot13,
message);
- return 0;
+ return {};
case TestInterface::NoArgs::Opcode:
DispatchRemoteMethod<TestInterface::NoArgs>(
*this, &TestService::OnNoArgs, message);
- return 0;
+ return {};
case TestInterface::SendFile::Opcode:
DispatchRemoteMethod<TestInterface::SendFile>(
*this, &TestService::OnSendFile, message);
- return 0;
+ return {};
case TestInterface::GetFile::Opcode:
DispatchRemoteMethod<TestInterface::GetFile>(
*this, &TestService::OnGetFile, message);
- return 0;
+ return {};
case TestInterface::GetTestFdType::Opcode:
DispatchRemoteMethod<TestInterface::GetTestFdType>(
*this, &TestService::OnGetTestFdType, message);
- return 0;
+ return {};
case TestInterface::OpenFiles::Opcode:
DispatchRemoteMethod<TestInterface::OpenFiles>(
*this, &TestService::OnOpenFiles, message);
- return 0;
+ return {};
case TestInterface::ReadFile::Opcode:
DispatchRemoteMethod<TestInterface::ReadFile>(
*this, &TestService::OnReadFile, message);
- return 0;
+ return {};
case TestInterface::PushChannel::Opcode:
DispatchRemoteMethod<TestInterface::PushChannel>(
*this, &TestService::OnPushChannel, message);
- return 0;
+ return {};
case TestInterface::Positive::Opcode:
DispatchRemoteMethod<TestInterface::Positive>(
*this, &TestService::OnPositive, message);
- return 0;
+ return {};
default:
return Service::DefaultHandleMessage(message);
@@ -433,7 +433,8 @@
friend BASE;
TestService()
- : BASE("TestService", Endpoint::Create(TestInterface::kClientPath)) {}
+ : BASE("TestService",
+ Endpoint::CreateAndBindSocket(TestInterface::kClientPath)) {}
int OnAdd(Message&, int a, int b) { return a + b; }
diff --git a/libs/vr/libpdx_uds/service_endpoint.cpp b/libs/vr/libpdx_uds/service_endpoint.cpp
index 7bf753d..f89b8a8 100644
--- a/libs/vr/libpdx_uds/service_endpoint.cpp
+++ b/libs/vr/libpdx_uds/service_endpoint.cpp
@@ -19,6 +19,7 @@
using android::pdx::BorrowedChannelHandle;
using android::pdx::BorrowedHandle;
using android::pdx::ChannelReference;
+using android::pdx::ErrorStatus;
using android::pdx::FileReference;
using android::pdx::LocalChannelHandle;
using android::pdx::LocalHandle;
@@ -51,14 +52,14 @@
return true;
}
- FileReference PushFileHandle(BorrowedHandle handle) {
+ Status<FileReference> PushFileHandle(BorrowedHandle handle) {
if (!handle)
return handle.Get();
response.file_descriptors.push_back(std::move(handle));
return response.file_descriptors.size() - 1;
}
- ChannelReference PushChannelHandle(BorrowedChannelHandle handle) {
+ Status<ChannelReference> PushChannelHandle(BorrowedChannelHandle handle) {
if (!handle)
return handle.value();
@@ -70,14 +71,14 @@
response.channels.push_back(std::move(channel_info));
return response.channels.size() - 1;
} else {
- return -1;
+ return ErrorStatus{EINVAL};
}
}
- ChannelReference PushChannelHandle(BorrowedHandle data_fd,
- BorrowedHandle event_fd) {
+ Status<ChannelReference> PushChannelHandle(BorrowedHandle data_fd,
+ BorrowedHandle event_fd) {
if (!data_fd || !event_fd)
- return -1;
+ return ErrorStatus{EINVAL};
ChannelInfo<BorrowedHandle> channel_info;
channel_info.data_fd = std::move(data_fd);
channel_info.event_fd = std::move(event_fd);
@@ -85,8 +86,8 @@
return response.channels.size() - 1;
}
- ssize_t WriteData(const iovec* vector, size_t vector_length) {
- ssize_t size = 0;
+ Status<size_t> WriteData(const iovec* vector, size_t vector_length) {
+ size_t size = 0;
for (size_t i = 0; i < vector_length; i++) {
const auto* data = reinterpret_cast<const uint8_t*>(vector[i].iov_base);
response_data.insert(response_data.end(), data, data + vector[i].iov_len);
@@ -95,9 +96,9 @@
return size;
}
- ssize_t ReadData(const iovec* vector, size_t vector_length) {
+ Status<size_t> ReadData(const iovec* vector, size_t vector_length) {
size_t size_remaining = request_data.size() - request_data_read_pos;
- ssize_t size = 0;
+ size_t size = 0;
for (size_t i = 0; i < vector_length && size_remaining > 0; i++) {
size_t size_to_copy = std::min(size_remaining, vector[i].iov_len);
memcpy(vector[i].iov_base, request_data.data() + request_data_read_pos,
@@ -214,18 +215,18 @@
return status;
}
-int Endpoint::SetService(Service* service) {
+Status<void> Endpoint::SetService(Service* service) {
service_ = service;
- return 0;
+ return {};
}
-int Endpoint::SetChannel(int channel_id, Channel* channel) {
+Status<void> Endpoint::SetChannel(int channel_id, Channel* channel) {
std::lock_guard<std::mutex> autolock(channel_mutex_);
auto channel_data = channels_.find(channel_id);
if (channel_data == channels_.end())
- return -EINVAL;
+ return ErrorStatus{EINVAL};
channel_data->second.channel_state = channel;
- return 0;
+ return {};
}
Status<void> Endpoint::OnNewChannel(LocalHandle channel_fd) {
@@ -269,44 +270,46 @@
return {};
}
-int Endpoint::CloseChannel(int channel_id) {
+Status<void> Endpoint::CloseChannel(int channel_id) {
std::lock_guard<std::mutex> autolock(channel_mutex_);
return CloseChannelLocked(channel_id);
}
-int Endpoint::CloseChannelLocked(int channel_id) {
+Status<void> Endpoint::CloseChannelLocked(int channel_id) {
ALOGD_IF(TRACE, "Endpoint::CloseChannelLocked: channel_id=%d", channel_id);
auto channel_data = channels_.find(channel_id);
if (channel_data == channels_.end())
- return -EINVAL;
+ return ErrorStatus{EINVAL};
- int ret = 0;
+ Status<void> status;
epoll_event dummy; // See BUGS in man 2 epoll_ctl.
if (epoll_ctl(epoll_fd_.Get(), EPOLL_CTL_DEL, channel_id, &dummy) < 0) {
- ret = -errno;
+ status.SetError(errno);
ALOGE(
"Endpoint::CloseChannelLocked: Failed to remove channel from endpoint: "
"%s\n",
strerror(errno));
+ } else {
+ status.SetValue();
}
channels_.erase(channel_data);
- return ret;
+ return status;
}
-int Endpoint::ModifyChannelEvents(int channel_id, int clear_mask,
- int set_mask) {
+Status<void> Endpoint::ModifyChannelEvents(int channel_id, int clear_mask,
+ int set_mask) {
std::lock_guard<std::mutex> autolock(channel_mutex_);
auto search = channels_.find(channel_id);
if (search != channels_.end()) {
auto& channel_data = search->second;
channel_data.event_set.ModifyEvents(clear_mask, set_mask);
- return 0;
+ return {};
}
- return -EINVAL;
+ return ErrorStatus{EINVAL};
}
Status<RemoteChannelHandle> Endpoint::PushChannel(Message* message,
@@ -337,17 +340,19 @@
*channel_id = local_socket.Get();
auto channel_data = OnNewChannelLocked(std::move(local_socket), channel);
if (!channel_data)
- return ErrorStatus(channel_data.error());
+ return channel_data.error_status();
// Flags are ignored for now.
// TODO(xiaohuit): Implement those.
auto* state = static_cast<MessageState*>(message->GetState());
- ChannelReference ref = state->PushChannelHandle(
+ Status<ChannelReference> ref = state->PushChannelHandle(
remote_socket.Borrow(),
channel_data.get()->event_set.event_fd().Borrow());
+ if (!ref)
+ return ref.error_status();
state->sockets_to_close.push_back(std::move(remote_socket));
- return RemoteChannelHandle{ref};
+ return RemoteChannelHandle{ref.get()};
}
Status<int> Endpoint::CheckChannel(const Message* /*message*/,
@@ -357,13 +362,6 @@
return ErrorStatus(EFAULT);
}
-int Endpoint::DefaultHandleMessage(const MessageInfo& /* info */) {
- ALOGE(
- "Endpoint::CheckChannel: Not implemented! Endpoint DefaultHandleMessage "
- "does nothing!");
- return 0;
-}
-
Channel* Endpoint::GetChannelState(int channel_id) {
std::lock_guard<std::mutex> autolock(channel_mutex_);
auto channel_data = channels_.find(channel_id);
@@ -464,7 +462,7 @@
*message = Message{info};
}
-int Endpoint::MessageReceive(Message* message) {
+Status<void> Endpoint::MessageReceive(Message* message) {
// Receive at most one event from the epoll set. This should prevent multiple
// dispatch threads from attempting to handle messages on the same socket at
// the same time.
@@ -474,40 +472,36 @@
if (count < 0) {
ALOGE("Endpoint::MessageReceive: Failed to wait for epoll events: %s\n",
strerror(errno));
- return -errno;
+ return ErrorStatus{errno};
} else if (count == 0) {
- return -ETIMEDOUT;
+ return ErrorStatus{ETIMEDOUT};
}
if (event.data.fd == cancel_event_fd_.Get()) {
- return -ESHUTDOWN;
+ return ErrorStatus{ESHUTDOWN};
}
if (event.data.fd == socket_fd_.Get()) {
auto status = AcceptConnection(message);
if (!status)
- return -status.error();
- status = ReenableEpollEvent(socket_fd_.Get());
- return status ? 0 : -status.error();
+ return status;
+ return ReenableEpollEvent(socket_fd_.Get());
}
int channel_id = event.data.fd;
if (event.events & (EPOLLRDHUP | EPOLLHUP)) {
BuildCloseMessage(channel_id, message);
- return 0;
+ return {};
}
- auto status = ReceiveMessageForChannel(channel_id, message);
- if (!status)
- return -status.error();
- return 0;
+ return ReceiveMessageForChannel(channel_id, message);
}
-int Endpoint::MessageReply(Message* message, int return_code) {
+Status<void> Endpoint::MessageReply(Message* message, int return_code) {
const int channel_id = message->GetChannelId();
const int channel_socket = GetChannelSocketFd(channel_id);
if (channel_socket < 0)
- return -EBADF;
+ return ErrorStatus{EBADF};
auto* state = static_cast<MessageState*>(message->GetState());
switch (message->GetOp()) {
@@ -515,12 +509,17 @@
return CloseChannel(channel_id);
case opcodes::CHANNEL_OPEN:
- if (return_code < 0)
+ if (return_code < 0) {
return CloseChannel(channel_id);
- // Reply with the event fd.
- return_code = state->PushFileHandle(
- BorrowedHandle{GetChannelEventFd(channel_socket)});
- state->response_data.clear(); // Just in case...
+ } else {
+ // Reply with the event fd.
+ auto push_status = state->PushFileHandle(
+ BorrowedHandle{GetChannelEventFd(channel_socket)});
+ state->response_data.clear(); // Just in case...
+ if (!push_status)
+ return push_status.error_status();
+ return_code = push_status.get();
+ }
break;
}
@@ -535,76 +534,82 @@
if (status)
status = ReenableEpollEvent(channel_socket);
- return status ? 0 : -status.error();
+ return status;
}
-int Endpoint::MessageReplyFd(Message* message, unsigned int push_fd) {
+Status<void> Endpoint::MessageReplyFd(Message* message, unsigned int push_fd) {
auto* state = static_cast<MessageState*>(message->GetState());
auto ref = state->PushFileHandle(BorrowedHandle{static_cast<int>(push_fd)});
- return MessageReply(message, ref);
+ if (!ref)
+ return ref.error_status();
+ return MessageReply(message, ref.get());
}
-int Endpoint::MessageReplyChannelHandle(Message* message,
- const LocalChannelHandle& handle) {
+Status<void> Endpoint::MessageReplyChannelHandle(
+ Message* message, const LocalChannelHandle& handle) {
auto* state = static_cast<MessageState*>(message->GetState());
auto ref = state->PushChannelHandle(handle.Borrow());
- return MessageReply(message, ref);
+ if (!ref)
+ return ref.error_status();
+ return MessageReply(message, ref.get());
}
-int Endpoint::MessageReplyChannelHandle(Message* message,
- const BorrowedChannelHandle& handle) {
+Status<void> Endpoint::MessageReplyChannelHandle(
+ Message* message, const BorrowedChannelHandle& handle) {
auto* state = static_cast<MessageState*>(message->GetState());
auto ref = state->PushChannelHandle(handle.Duplicate());
- return MessageReply(message, ref);
+ if (!ref)
+ return ref.error_status();
+ return MessageReply(message, ref.get());
}
-int Endpoint::MessageReplyChannelHandle(Message* message,
- const RemoteChannelHandle& handle) {
+Status<void> Endpoint::MessageReplyChannelHandle(
+ Message* message, const RemoteChannelHandle& handle) {
return MessageReply(message, handle.value());
}
-ssize_t Endpoint::ReadMessageData(Message* message, const iovec* vector,
- size_t vector_length) {
+Status<size_t> Endpoint::ReadMessageData(Message* message, const iovec* vector,
+ size_t vector_length) {
auto* state = static_cast<MessageState*>(message->GetState());
return state->ReadData(vector, vector_length);
}
-ssize_t Endpoint::WriteMessageData(Message* message, const iovec* vector,
- size_t vector_length) {
+Status<size_t> Endpoint::WriteMessageData(Message* message, const iovec* vector,
+ size_t vector_length) {
auto* state = static_cast<MessageState*>(message->GetState());
return state->WriteData(vector, vector_length);
}
-FileReference Endpoint::PushFileHandle(Message* message,
- const LocalHandle& handle) {
+Status<FileReference> Endpoint::PushFileHandle(Message* message,
+ const LocalHandle& handle) {
auto* state = static_cast<MessageState*>(message->GetState());
return state->PushFileHandle(handle.Borrow());
}
-FileReference Endpoint::PushFileHandle(Message* message,
- const BorrowedHandle& handle) {
+Status<FileReference> Endpoint::PushFileHandle(Message* message,
+ const BorrowedHandle& handle) {
auto* state = static_cast<MessageState*>(message->GetState());
return state->PushFileHandle(handle.Duplicate());
}
-FileReference Endpoint::PushFileHandle(Message* /*message*/,
- const RemoteHandle& handle) {
+Status<FileReference> Endpoint::PushFileHandle(Message* /*message*/,
+ const RemoteHandle& handle) {
return handle.Get();
}
-ChannelReference Endpoint::PushChannelHandle(Message* message,
- const LocalChannelHandle& handle) {
+Status<ChannelReference> Endpoint::PushChannelHandle(
+ Message* message, const LocalChannelHandle& handle) {
auto* state = static_cast<MessageState*>(message->GetState());
return state->PushChannelHandle(handle.Borrow());
}
-ChannelReference Endpoint::PushChannelHandle(
+Status<ChannelReference> Endpoint::PushChannelHandle(
Message* message, const BorrowedChannelHandle& handle) {
auto* state = static_cast<MessageState*>(message->GetState());
return state->PushChannelHandle(handle.Duplicate());
}
-ChannelReference Endpoint::PushChannelHandle(
+Status<ChannelReference> Endpoint::PushChannelHandle(
Message* /*message*/, const RemoteChannelHandle& handle) {
return handle.value();
}
@@ -624,8 +629,10 @@
return handle;
}
-int Endpoint::Cancel() {
- return (eventfd_write(cancel_event_fd_.Get(), 1) < 0) ? -errno : 0;
+Status<void> Endpoint::Cancel() {
+ if (eventfd_write(cancel_event_fd_.Get(), 1) < 0)
+ return ErrorStatus{errno};
+ return {};
}
std::unique_ptr<Endpoint> Endpoint::Create(const std::string& endpoint_path,
@@ -634,6 +641,14 @@
return std::unique_ptr<Endpoint>(new Endpoint(endpoint_path, blocking));
}
+std::unique_ptr<Endpoint> Endpoint::CreateAndBindSocket(
+ const std::string& endpoint_path, bool blocking) {
+ // TODO(avakulenko): When Endpoint can differentiate between absolute paths
+ // and relative paths/socket names created by the init process, change this
+ // code to reflect the fact that we want to use absolute paths here.
+ return std::unique_ptr<Endpoint>(new Endpoint(endpoint_path, blocking));
+}
+
} // namespace uds
} // namespace pdx
} // namespace android
diff --git a/libs/vr/libpdx_uds/service_framework_tests.cpp b/libs/vr/libpdx_uds/service_framework_tests.cpp
index 9e31e82..2943239 100644
--- a/libs/vr/libpdx_uds/service_framework_tests.cpp
+++ b/libs/vr/libpdx_uds/service_framework_tests.cpp
@@ -119,106 +119,106 @@
}
}
- int HandleMessage(Message& message) override {
+ Status<void> HandleMessage(Message& message) override {
switch (message.GetOp()) {
case TEST_OP_GET_SERVICE_ID:
- REPLY_MESSAGE_RETURN(message, service_id_, 0);
+ REPLY_MESSAGE_RETURN(message, service_id_, {});
// Set the test channel to the TestChannel for the current channel. Other
// messages can use this to perform tests.
case TEST_OP_SET_TEST_CHANNEL:
test_channel_ = message.GetChannel<TestChannel>();
- REPLY_MESSAGE_RETURN(message, 0, 0);
+ REPLY_MESSAGE_RETURN(message, 0, {});
// Return the channel id for the current channel.
case TEST_OP_GET_THIS_CHANNEL_ID:
- REPLY_MESSAGE_RETURN(message, message.GetChannelId(), 0);
+ REPLY_MESSAGE_RETURN(message, message.GetChannelId(), {});
// Return the channel id for the test channel.
case TEST_OP_GET_TEST_CHANNEL_ID:
if (test_channel_)
- REPLY_MESSAGE_RETURN(message, test_channel_->channel_id(), 0);
+ REPLY_MESSAGE_RETURN(message, test_channel_->channel_id(), {});
else
- REPLY_ERROR_RETURN(message, ENOENT, 0);
+ REPLY_ERROR_RETURN(message, ENOENT, {});
// Test check channel feature.
case TEST_OP_CHECK_CHANNEL_ID: {
ChannelReference ref = 0;
- if (message.Read(&ref, sizeof(ref)) < static_cast<ssize_t>(sizeof(ref)))
- REPLY_ERROR_RETURN(message, EIO, 0);
+ if (!message.ReadAll(&ref, sizeof(ref)))
+ REPLY_ERROR_RETURN(message, EIO, {});
const Status<int> ret = message.CheckChannel<TestChannel>(ref, nullptr);
- REPLY_MESSAGE_RETURN(message, ret, 0);
+ REPLY_MESSAGE_RETURN(message, ret, {});
}
case TEST_OP_CHECK_CHANNEL_OBJECT: {
std::shared_ptr<TestChannel> channel;
ChannelReference ref = 0;
- if (message.Read(&ref, sizeof(ref)) < static_cast<ssize_t>(sizeof(ref)))
- REPLY_ERROR_RETURN(message, EIO, 0);
+ if (!message.ReadAll(&ref, sizeof(ref)))
+ REPLY_ERROR_RETURN(message, EIO, {});
const Status<int> ret =
message.CheckChannel<TestChannel>(ref, &channel);
if (!ret)
- REPLY_MESSAGE_RETURN(message, ret, 0);
+ REPLY_MESSAGE_RETURN(message, ret, {});
if (channel != nullptr)
- REPLY_MESSAGE_RETURN(message, channel->channel_id(), 0);
+ REPLY_MESSAGE_RETURN(message, channel->channel_id(), {});
else
- REPLY_ERROR_RETURN(message, ENODATA, 0);
+ REPLY_ERROR_RETURN(message, ENODATA, {});
}
case TEST_OP_CHECK_CHANNEL_FROM_OTHER_SERVICE: {
ChannelReference ref = 0;
- if (message.Read(&ref, sizeof(ref)) < static_cast<ssize_t>(sizeof(ref)))
- REPLY_ERROR_RETURN(message, EIO, 0);
+ if (!message.ReadAll(&ref, sizeof(ref)))
+ REPLY_ERROR_RETURN(message, EIO, {});
const Status<int> ret = message.CheckChannel<TestChannel>(
other_service_.get(), ref, nullptr);
- REPLY_MESSAGE_RETURN(message, ret, 0);
+ REPLY_MESSAGE_RETURN(message, ret, {});
}
case TEST_OP_GET_NEW_CHANNEL: {
auto channel = std::make_shared<TestChannel>(-1);
Status<RemoteChannelHandle> channel_handle =
message.PushChannel(0, channel, &channel->channel_id_);
- REPLY_MESSAGE_RETURN(message, channel_handle, 0);
+ REPLY_MESSAGE_RETURN(message, channel_handle, {});
}
case TEST_OP_GET_NEW_CHANNEL_FROM_OTHER_SERVICE: {
if (!other_service_)
- REPLY_ERROR_RETURN(message, EINVAL, 0);
+ REPLY_ERROR_RETURN(message, EINVAL, {});
auto channel = std::make_shared<TestChannel>(-1);
Status<RemoteChannelHandle> channel_handle = message.PushChannel(
other_service_.get(), 0, channel, &channel->channel_id_);
- REPLY_MESSAGE_RETURN(message, channel_handle, 0);
+ REPLY_MESSAGE_RETURN(message, channel_handle, {});
}
case TEST_OP_GET_THIS_PROCESS_ID:
- REPLY_MESSAGE_RETURN(message, message.GetProcessId(), 0);
+ REPLY_MESSAGE_RETURN(message, message.GetProcessId(), {});
case TEST_OP_GET_THIS_THREAD_ID:
- REPLY_MESSAGE_RETURN(message, message.GetThreadId(), 0);
+ REPLY_MESSAGE_RETURN(message, message.GetThreadId(), {});
case TEST_OP_GET_THIS_EUID:
- REPLY_MESSAGE_RETURN(message, message.GetEffectiveUserId(), 0);
+ REPLY_MESSAGE_RETURN(message, message.GetEffectiveUserId(), {});
case TEST_OP_GET_THIS_EGID:
- REPLY_MESSAGE_RETURN(message, message.GetEffectiveGroupId(), 0);
+ REPLY_MESSAGE_RETURN(message, message.GetEffectiveGroupId(), {});
case TEST_OP_POLLIN_FROM_SERVICE:
REPLY_MESSAGE_RETURN(message, message.ModifyChannelEvents(0, EPOLLIN),
- 0);
+ {});
case TEST_OP_SEND_LARGE_DATA_RETURN_SUM: {
std::array<int, kLargeDataSize> data_array;
- ssize_t size_to_read = data_array.size() * sizeof(int);
- ssize_t read = message.Read(data_array.data(), size_to_read);
- if (read < size_to_read)
- REPLY_ERROR_RETURN(message, EIO, 0);
+ size_t size_to_read = data_array.size() * sizeof(int);
+ if (!message.ReadAll(data_array.data(), size_to_read)) {
+ REPLY_ERROR_RETURN(message, EIO, {});
+ }
int sum = std::accumulate(data_array.begin(), data_array.end(), 0);
- REPLY_MESSAGE_RETURN(message, sum, 0);
+ REPLY_MESSAGE_RETURN(message, sum, {});
}
default:
@@ -245,7 +245,7 @@
TestService(const std::string& name,
const std::shared_ptr<TestService>& other_service, bool blocking)
: BASE(std::string("TestService") + name,
- Endpoint::Create(kTestServicePath + name, blocking)),
+ Endpoint::CreateAndBindSocket(kTestServicePath + name, blocking)),
other_service_(other_service),
service_id_(service_counter_++) {}
@@ -300,7 +300,7 @@
// Returns the channel id of the channel.
int CheckChannelIdArgument(BorrowedChannelHandle channel) {
Transaction trans{*this};
- ChannelReference ref = trans.PushChannelHandle(channel);
+ ChannelReference ref = trans.PushChannelHandle(channel).get();
return ReturnStatusOrError(trans.Send<int>(TEST_OP_CHECK_CHANNEL_ID, &ref,
sizeof(ref), nullptr, 0));
}
@@ -309,7 +309,7 @@
// Returns the channel id of the channel exercising the context pointer.
int CheckChannelObjectArgument(BorrowedChannelHandle channel) {
Transaction trans{*this};
- ChannelReference ref = trans.PushChannelHandle(channel);
+ ChannelReference ref = trans.PushChannelHandle(channel).get();
return ReturnStatusOrError(trans.Send<int>(TEST_OP_CHECK_CHANNEL_OBJECT,
&ref, sizeof(ref), nullptr, 0));
}
@@ -318,7 +318,7 @@
// Returns 0 on success.
int CheckChannelFromOtherService(BorrowedChannelHandle channel) {
Transaction trans{*this};
- ChannelReference ref = trans.PushChannelHandle(channel);
+ ChannelReference ref = trans.PushChannelHandle(channel).get();
return ReturnStatusOrError(
trans.Send<int>(TEST_OP_CHECK_CHANNEL_FROM_OTHER_SERVICE, &ref,
sizeof(ref), nullptr, 0));