From 4d7aae5eb2d5214b09dc20a1ec0542057df87139 Mon Sep 17 00:00:00 2001 From: Joel Winarske Date: Sun, 12 Jul 2026 16:36:24 -0700 Subject: [PATCH] Add std::expected syscall wrappers and adopt them for fd creation Introduce src/utils/sys.h: thin std::expected wrappers over the fd-creating syscalls (open, eventfd, signalfd, timerfd). A fallible call now hands back a [[nodiscard]], allocation-free error channel (errno mapped to std::generic_category()) with the fd already owned, instead of a bare -1 the caller must remember to check. This is the errno layer only, and it establishes the error-model convention for the project: syscalls -> std::expected; D-Bus failures -> sdbus::Error exceptions (sdbus-c++'s model); absent-but-valid values -> std::optional. Each is the right tool for its layer. Adopted at the fd-creation sites: EventLoop's wake eventfd, SignalSource's signalfd, and the three controllers' hidraw open() (which now also logs the specific errno message). An opt-in unit test (sys_test) covers success, the ENOENT error path, and the eventfd/timerfd wrappers. Further adoption (e.g. the hidraw ioctl chain) can follow the same shape; this change lands the foundation and the reference call sites. Signed-off-by: Joel Winarske --- src/bluez/horipad_steam/input_reader.cc | 8 +- src/bluez/ps5_dual_sense/input_reader.cc | 8 +- src/bluez/xbox_controller/input_reader.cc | 8 +- src/utils/CMakeLists.txt | 5 ++ src/utils/event_loop.cc | 9 ++- src/utils/signal_source.h | 8 +- src/utils/sys.h | 97 +++++++++++++++++++++++ src/utils/sys_test.cc | 66 +++++++++++++++ 8 files changed, 194 insertions(+), 15 deletions(-) create mode 100644 src/utils/sys.h create mode 100644 src/utils/sys_test.cc diff --git a/src/bluez/horipad_steam/input_reader.cc b/src/bluez/horipad_steam/input_reader.cc index 55840ea..d8d42cc 100644 --- a/src/bluez/horipad_steam/input_reader.cc +++ b/src/bluez/horipad_steam/input_reader.cc @@ -21,6 +21,7 @@ #include #include "../../utils/logging.h" +#include "../../utils/sys.h" #include "../hidraw.hpp" #include "input_reader.h" @@ -32,11 +33,12 @@ void InputReader::open_and_init() { LOG_DEBUG("hidraw device: {}", device_); // Non-blocking so dispatch()'s read() never stalls the event loop. - UniqueFd fd(open(device_.c_str(), O_RDWR | O_NONBLOCK | O_CLOEXEC)); - if (!fd.valid()) { - LOG_ERROR("unable to open device"); + auto opened = sys::open_fd(device_.c_str(), O_RDWR | O_NONBLOCK | O_CLOEXEC); + if (!opened) { + LOG_ERROR("unable to open device: {}", opened.error().message()); return; } + UniqueFd fd = std::move(*opened); // Raw Info hidraw_devinfo raw_dev_info{}; diff --git a/src/bluez/ps5_dual_sense/input_reader.cc b/src/bluez/ps5_dual_sense/input_reader.cc index e21044d..3ebf0fb 100644 --- a/src/bluez/ps5_dual_sense/input_reader.cc +++ b/src/bluez/ps5_dual_sense/input_reader.cc @@ -22,6 +22,7 @@ #include #include "../../utils/logging.h" +#include "../../utils/sys.h" #include "../hidraw.hpp" #include "input_reader.h" @@ -33,11 +34,12 @@ void InputReader::open_and_init() { LOG_DEBUG("hidraw device: {}", device_); // Non-blocking so dispatch()'s read() never stalls the event loop. - UniqueFd fd(open(device_.c_str(), O_RDWR | O_NONBLOCK | O_CLOEXEC)); - if (!fd.valid()) { - LOG_ERROR("unable to open device"); + auto opened = sys::open_fd(device_.c_str(), O_RDWR | O_NONBLOCK | O_CLOEXEC); + if (!opened) { + LOG_ERROR("unable to open device: {}", opened.error().message()); return; } + UniqueFd fd = std::move(*opened); // Raw Info hidraw_devinfo raw_dev_info{}; diff --git a/src/bluez/xbox_controller/input_reader.cc b/src/bluez/xbox_controller/input_reader.cc index 2baacf4..57b4976 100644 --- a/src/bluez/xbox_controller/input_reader.cc +++ b/src/bluez/xbox_controller/input_reader.cc @@ -21,6 +21,7 @@ #include #include "../../utils/logging.h" +#include "../../utils/sys.h" #include "../hidraw.hpp" #include "input_reader.h" @@ -32,11 +33,12 @@ void InputReader::open_and_init() { LOG_DEBUG("hidraw device: {}", device_); // Non-blocking so dispatch()'s read() never stalls the event loop. - UniqueFd fd(open(device_.c_str(), O_RDWR | O_NONBLOCK | O_CLOEXEC)); - if (!fd.valid()) { - LOG_ERROR("unable to open device"); + auto opened = sys::open_fd(device_.c_str(), O_RDWR | O_NONBLOCK | O_CLOEXEC); + if (!opened) { + LOG_ERROR("unable to open device: {}", opened.error().message()); return; } + UniqueFd fd = std::move(*opened); // Raw Info hidraw_devinfo raw_dev_info{}; diff --git a/src/utils/CMakeLists.txt b/src/utils/CMakeLists.txt index 4dd2a9e..5d14f3e 100644 --- a/src/utils/CMakeLists.txt +++ b/src/utils/CMakeLists.txt @@ -2,6 +2,7 @@ add_library(utils STATIC event_loop.cc event_loop.h + sys.h utils.cc utils.h ) @@ -16,4 +17,8 @@ if (SDBUS_CPP_EXAMPLES_BUILD_TESTS) add_executable(event_loop_test event_loop_test.cc) target_link_libraries(event_loop_test PRIVATE utils sdbus-c++ spdlog::spdlog) add_test(NAME event_loop_test COMMAND event_loop_test) + + add_executable(sys_test sys_test.cc) + target_link_libraries(sys_test PRIVATE utils spdlog::spdlog) + add_test(NAME sys_test COMMAND sys_test) endif () diff --git a/src/utils/event_loop.cc b/src/utils/event_loop.cc index 7fef2db..5c79e7e 100644 --- a/src/utils/event_loop.cc +++ b/src/utils/event_loop.cc @@ -25,14 +25,17 @@ #include #include "logging.h" +#include "sys.h" namespace { constexpr std::size_t kNotPresent = static_cast(-1); } // namespace -EventLoop::EventLoop() : wake_fd_(::eventfd(0, EFD_CLOEXEC | EFD_NONBLOCK)) { - if (!wake_fd_.valid()) { - LOG_ERROR("EventLoop: failed to create eventfd: {}", strerror(errno)); +EventLoop::EventLoop() { + if (auto fd = sys::make_eventfd(0, EFD_CLOEXEC | EFD_NONBLOCK)) { + wake_fd_ = std::move(*fd); + } else { + LOG_ERROR("EventLoop: failed to create eventfd: {}", fd.error().message()); } } diff --git a/src/utils/signal_source.h b/src/utils/signal_source.h index fb75258..22553cc 100644 --- a/src/utils/signal_source.h +++ b/src/utils/signal_source.h @@ -25,6 +25,7 @@ #include "event_loop.h" #include "logging.h" +#include "sys.h" #include "unique_fd.h" /// Turns POSIX signals into an EventLoop stop(). The signals are blocked @@ -45,12 +46,13 @@ class SignalSource final : public EventSource { if (sigprocmask(SIG_BLOCK, &mask_, nullptr) < 0) { LOG_ERROR("SignalSource: sigprocmask failed: {}", strerror(errno)); } - fd_ = UniqueFd(::signalfd(-1, &mask_, SFD_CLOEXEC | SFD_NONBLOCK)); - if (!fd_.valid()) { + if (auto fd = sys::make_signalfd(-1, mask_, SFD_CLOEXEC | SFD_NONBLOCK)) { + fd_ = std::move(*fd); + } else { LOG_ERROR( "SignalSource: signalfd failed: {}; restoring default signal " "disposition", - strerror(errno)); + fd.error().message()); // Undo the block, otherwise the signals stay blocked with no consumer and // the process can no longer be stopped via SIGINT/SIGTERM. sigprocmask(SIG_UNBLOCK, &mask_, nullptr); diff --git a/src/utils/sys.h b/src/utils/sys.h new file mode 100644 index 0000000..0be4ddf --- /dev/null +++ b/src/utils/sys.h @@ -0,0 +1,97 @@ +// Copyright (c) 2026 Joel Winarske +// +// Licensed under the Apache License, Version 2.0 (the "License"); +// you may not use this file except in compliance with the License. +// You may obtain a copy of the License at +// +// http://www.apache.org/licenses/LICENSE-2.0 +// +// Unless required by applicable law or agreed to in writing, software +// distributed under the License is distributed on an "AS IS" BASIS, +// WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied. +// See the License for the specific language governing permissions and +// limitations under the License. + +#ifndef SRC_UTILS_SYS_H +#define SRC_UTILS_SYS_H + +#include +#include +#include + +#include +#include +#include +#include +#include + +#include "unique_fd.h" + +/// Thin std::expected wrappers over the fd-creating syscalls. +/// +/// Fallible system calls return std::expected so a +/// caller gets a [[nodiscard]], allocation-free error channel — errno mapped to +/// std::generic_category() — instead of a bare -1 it must remember to check, +/// and the fd is owned (closed on scope exit) the moment it is produced. +/// +/// This is the errno layer only. D-Bus failures still surface as sdbus::Error +/// exceptions (sdbus-c++'s model), and absent-but-not-erroneous values still +/// use std::optional; both are the right tool for their layer. +namespace sys { + +/// The current errno as a std::error_code. +[[nodiscard]] inline std::error_code last_error() noexcept { + return {errno, std::generic_category()}; +} + +[[nodiscard]] inline std::expected open_fd( + const char* path, + const int flags) noexcept { + UniqueFd fd(::open(path, flags)); + if (!fd.valid()) { + return std::unexpected(last_error()); + } + return fd; +} + +[[nodiscard]] inline std::expected +open_fd(const char* path, const int flags, const mode_t mode) noexcept { + UniqueFd fd(::open(path, flags, mode)); + if (!fd.valid()) { + return std::unexpected(last_error()); + } + return fd; +} + +[[nodiscard]] inline std::expected make_eventfd( + const unsigned initval, + const int flags) noexcept { + UniqueFd fd(::eventfd(initval, flags)); + if (!fd.valid()) { + return std::unexpected(last_error()); + } + return fd; +} + +[[nodiscard]] inline std::expected +make_signalfd(const int fd, const sigset_t& mask, const int flags) noexcept { + UniqueFd out(::signalfd(fd, &mask, flags)); + if (!out.valid()) { + return std::unexpected(last_error()); + } + return out; +} + +[[nodiscard]] inline std::expected make_timerfd( + const int clockid, + const int flags) noexcept { + UniqueFd fd(::timerfd_create(clockid, flags)); + if (!fd.valid()) { + return std::unexpected(last_error()); + } + return fd; +} + +} // namespace sys + +#endif // SRC_UTILS_SYS_H diff --git a/src/utils/sys_test.cc b/src/utils/sys_test.cc new file mode 100644 index 0000000..0092d56 --- /dev/null +++ b/src/utils/sys_test.cc @@ -0,0 +1,66 @@ +// Copyright (c) 2026 Joel Winarske +// +// Licensed under the Apache License, Version 2.0 (the "License"); +// you may not use this file except in compliance with the License. +// You may obtain a copy of the License at +// +// http://www.apache.org/licenses/LICENSE-2.0 +// +// Unless required by applicable law or agreed to in writing, software +// distributed under the License is distributed on an "AS IS" BASIS, +// WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied. +// See the License for the specific language governing permissions and +// limitations under the License. + +// Self-test for the sys:: std::expected syscall wrappers: a success returns an +// owned, valid fd; a failure returns the expected std::error_code. + +#include + +#include +#include +#include + +#include "logging.h" +#include "sys.h" + +int main() { + int failures = 0; + const auto check = [&failures](const bool cond, const char* what) { + if (cond) { + LOG_INFO("sys test: {} - ok", what); + } else { + LOG_ERROR("sys test: {} - FAIL", what); + ++failures; + } + }; + + // open_fd success: /dev/null always exists and is readable. + if (auto fd = sys::open_fd("/dev/null", O_RDONLY | O_CLOEXEC)) { + check(fd->valid(), "open_fd(/dev/null) yields a valid fd"); + } else { + check(false, "open_fd(/dev/null) unexpectedly failed"); + } + + // open_fd failure: a missing path yields the matching error_code, no fd. + if (auto fd = sys::open_fd("/nonexistent/sdbus-cpp-examples/xyz", + O_RDONLY | O_CLOEXEC)) { + check(false, "open_fd(missing) unexpectedly succeeded"); + } else { + check(fd.error() == std::errc::no_such_file_or_directory, + "open_fd(missing) reports ENOENT"); + } + + // make_eventfd / make_timerfd success. + check(sys::make_eventfd(0, EFD_CLOEXEC | EFD_NONBLOCK).has_value(), + "make_eventfd yields a valid fd"); + check(sys::make_timerfd(CLOCK_MONOTONIC, TFD_CLOEXEC).has_value(), + "make_timerfd yields a valid fd"); + + if (failures == 0) { + LOG_INFO("sys test: PASS"); + return 0; + } + LOG_ERROR("sys test: FAIL ({} check(s))", failures); + return 1; +}