From 1db86c401df11423c945634d8b2a483e97afa878 Mon Sep 17 00:00:00 2001 From: Lexi Winter Date: Thu, 26 Jun 2025 18:58:44 +0100 Subject: nihil: improve error handling --- nihil.config/read.cc | 3 +- nihil/CMakeLists.txt | 1 + nihil/argv.cc | 65 +++++++++++++++++ nihil/argv.ccm | 73 ++++++------------ nihil/ensure_dir.cc | 15 ++-- nihil/ensure_dir.ccm | 9 ++- nihil/error.cc | 8 ++ nihil/error.ccm | 36 ++++++--- nihil/exec.cc | 57 +++++++-------- nihil/exec.ccm | 51 +++++-------- nihil/fd.cc | 105 +++++++++++--------------- nihil/fd.ccm | 45 +++++++----- nihil/getenv.cc | 5 +- nihil/getenv.ccm | 4 +- nihil/monad.ccm | 5 +- nihil/process.cc | 61 +++++++--------- nihil/process.ccm | 30 ++------ nihil/read_file.ccm | 33 ++++----- nihil/spawn.ccm | 203 +++++++++++++++++++++++++++++---------------------- nihil/tests/fd.cc | 9 +-- nihil/tests/spawn.cc | 92 ++++++++++++++++++----- 21 files changed, 490 insertions(+), 420 deletions(-) create mode 100644 nihil/argv.cc diff --git a/nihil.config/read.cc b/nihil.config/read.cc index 3c20566..0a5fcad 100644 --- a/nihil.config/read.cc +++ b/nihil.config/read.cc @@ -4,6 +4,7 @@ module; +#include #include #include #include @@ -27,7 +28,7 @@ auto read_from(std::filesystem::path const &filename) if (!err) { // Ignore ENOENT, it simply means we haven't created the // config file yet, so default values will be used. - if (err.error() == std::errc::no_such_file_or_directory) + if (err.error().root_cause() == std::errc::no_such_file_or_directory) return {}; auto errstr = std::format("cannot read {}", filename.string()); return std::unexpected(nihil::error(errstr, err.error())); diff --git a/nihil/CMakeLists.txt b/nihil/CMakeLists.txt index 73eea0b..fea785b 100644 --- a/nihil/CMakeLists.txt +++ b/nihil/CMakeLists.txt @@ -32,6 +32,7 @@ target_sources(nihil write_file.ccm PRIVATE + argv.cc command_map.cc ensure_dir.cc error.cc diff --git a/nihil/argv.cc b/nihil/argv.cc new file mode 100644 index 0000000..f38293f --- /dev/null +++ b/nihil/argv.cc @@ -0,0 +1,65 @@ +/* + * This source code is released into the public domain. + */ + +module; + +#include +#include +#include +#include + +module nihil; + +namespace nihil { + +argv::argv() = default; +argv::argv(argv &&) noexcept = default; +auto argv::operator=(this argv &, argv &&) -> argv & = default; + +argv::~argv() +{ + for (auto *arg : m_args) + delete[] arg; +} + +auto argv::data(this argv const &self) -> char const * const * +{ + return self.m_args.data(); +} + +auto argv::data(this argv &self) -> char * const * +{ + return self.m_args.data(); +} + +auto argv::size(this argv const &self) +{ + return self.m_args.size(); +} + +auto argv::begin(this argv const &self) +{ + return self.m_args.begin(); +} + +auto argv::end(this argv const &self) +{ + return self.m_args.end(); +} + + +auto argv::add_arg(this argv &self, std::string_view arg) -> void +{ + // Create a nul-terminated C string. + auto ptr = std::make_unique(arg.size() + 1); + std::ranges::copy(arg, ptr.get()); + ptr[arg.size()] = '\0'; + + // Ensure we won't throw when emplacing the pointer. + self.m_args.reserve(self.m_args.size() + 1); + self.m_args.emplace_back(ptr.release()); +} + +} // namespace nihil + diff --git a/nihil/argv.ccm b/nihil/argv.ccm index a9c254e..b5d6d6a 100644 --- a/nihil/argv.ccm +++ b/nihil/argv.ccm @@ -25,82 +25,53 @@ export struct argv { /* * Create a new argv from a range. */ - static auto from_range(std::ranges::range auto &&args) -> argv + argv(std::from_range_t, std::ranges::range auto &&args) { - auto ret = argv{}; - for (auto &&arg : args) - ret._add_arg(std::string_view(arg)); + add_arg(std::string_view(arg)); - ret._args.push_back(nullptr); - return ret; + m_args.push_back(nullptr); } + /* + * Create an argv from an initializer list. + */ template - static auto from_args(std::initializer_list &&args) + explicit argv(std::initializer_list &&args) + : argv(std::from_range, std::forward(args)) { - return from_range(std::move(args)); } - argv(argv &&) noexcept = default; - auto operator=(this argv &, argv &&other) -> argv& = default; + // Movable. + argv(argv &&) noexcept; + auto operator=(this argv &, argv &&other) -> argv &; // Not copyable. TODO: for completeness, it probably should be. argv(argv const &) = delete; auto operator=(this argv &, argv const &other) -> argv& = delete; - ~argv() - { - for (auto *arg : _args) - delete[] arg; - } + ~argv(); // Access the stored arguments. - auto data(this argv const &self) -> char const * const * - { - return self._args.data(); - } - - auto data(this argv &self) -> char * const * - { - return self._args.data(); - } - - auto size(this argv const &self) - { - return self._args.size(); - } + [[nodiscard]] auto data(this argv const &self) -> char const * const *; + [[nodiscard]] auto data(this argv &self) -> char * const *; + [[nodiscard]] auto size(this argv const &self); // Range access - auto begin(this argv const &self) - { - return self._args.begin(); - } - - auto end(this argv const &self) - { - return self._args.end(); - } + [[nodiscard]] auto begin(this argv const &self); + [[nodiscard]] auto end(this argv const &self); private: // Use the from_range() factory method to create new instances. - argv() = default; + argv(); // The argument pointers, including the null terminator. - std::vector _args; + // This can't be a vector because we need an array of + // char pointers to pass to exec. + std::vector m_args; // Add a new argument to the array. - auto _add_arg(this argv &self, std::string_view arg) -> void - { - // Create a nul-terminated C string. - auto ptr = std::make_unique(arg.size() + 1); - std::ranges::copy(arg, ptr.get()); - ptr[arg.size()] = '\0'; - - // Ensure we won't throw when emplacing the pointer. - self._args.reserve(self._args.size() + 1); - self._args.emplace_back(ptr.release()); - } + auto add_arg(this argv &self, std::string_view arg) -> void; }; } // namespace nihil diff --git a/nihil/ensure_dir.cc b/nihil/ensure_dir.cc index 019d268..f20d328 100644 --- a/nihil/ensure_dir.cc +++ b/nihil/ensure_dir.cc @@ -4,6 +4,7 @@ module; +#include #include #include #include @@ -12,16 +13,16 @@ module nihil; namespace nihil { -auto ensure_dir(std::filesystem::path const &dir) -> void +auto ensure_dir(std::filesystem::path const &dir) -> std::expected { - std::error_code err; - - if (std::filesystem::create_directories(dir, err)) - return; + auto err = std::error_code(); + std::filesystem::create_directories(dir, err); + if (err) - throw generic_error(std::format("{}: mkdir: {}", - dir.string(), err.message())); + return std::unexpected(error(err)); + + return {}; } } // namespace nihil diff --git a/nihil/ensure_dir.ccm b/nihil/ensure_dir.ccm index 6ea1b32..af2c4c7 100644 --- a/nihil/ensure_dir.ccm +++ b/nihil/ensure_dir.ccm @@ -4,17 +4,20 @@ module; +#include #include export module nihil:ensure_dir; +import :error; + namespace nihil { /* - * Create the given directory and any parent directories; throws - * generic_error on failure. + * Create the given directory and any parent directories. */ -export auto ensure_dir(std::filesystem::path const &dir) -> void; +export [[nodiscard]] auto ensure_dir(std::filesystem::path const &dir) + -> std::expected; } // namespace nihil diff --git a/nihil/error.cc b/nihil/error.cc index de05855..dba3021 100644 --- a/nihil/error.cc +++ b/nihil/error.cc @@ -62,6 +62,14 @@ auto error::cause(this error const &self) -> std::optional return {}; } +auto error::root_cause(this error const &self) -> error const & +{ + if (self.m_cause) + return self.m_cause->root_cause(); + + return self; +} + auto error::str(this error const &self) -> std::string { return self.m_error | match { diff --git a/nihil/error.ccm b/nihil/error.ccm index 4000cdb..7576480 100644 --- a/nihil/error.ccm +++ b/nihil/error.ccm @@ -97,19 +97,25 @@ export struct error { auto operator=(this error &, error &&) noexcept -> error &; // Return the cause of this error. - auto cause(this error const &) -> std::optional; + [[nodiscard]] auto cause(this error const &) -> std::optional; + + // Return the root cause of this error, which may be this object. + // For errors caused by an OS error, this will typically be the + // error_code error. + [[nodiscard]] auto root_cause(this error const &) -> error const &; // Format this error and its cause(s) as a string. - auto what(this error const &) -> std::string; + [[nodiscard]] auto what(this error const &) -> std::string; // Format this error as a string. - auto str(this error const &) -> std::string; + [[nodiscard]] auto str(this error const &) -> std::string; // Return this error's error_code, if any. - auto code(this error const &) -> std::optional; + [[nodiscard]] auto code(this error const &) + -> std::optional; // Return this error's error_condition, if any. - auto condition(this error const &) + [[nodiscard]] auto condition(this error const &) -> std::optional; private: @@ -123,31 +129,37 @@ private: // Compare an error to another error. This only compares the error itself, // not any nested causes. -export auto operator==(error const &, error const &) -> bool; -export auto operator<=>(error const &, error const &) -> std::strong_ordering; +export [[nodiscard]] auto operator==(error const &, error const &) + -> bool; +export [[nodiscard]] auto operator<=>(error const &, error const &) + -> std::strong_ordering; // Compare an error to an std::error_code. -export auto operator==(error const &, std::error_code const &) -> bool; +export [[nodiscard]] auto operator==(error const &, std::error_code const &) + -> bool; // Compare an error to an std::error_condition. -export auto operator==(error const &, std::error_condition const &) -> bool; +export [[nodiscard]] auto operator==(error const &, + std::error_condition const &) + -> bool; // Compare an error to an std::error_code enum. -export auto operator==(error const &lhs, auto rhs) -> bool +export [[nodiscard]] auto operator==(error const &lhs, auto rhs) -> bool requires(std::is_error_code_enum::value) { return lhs.code() == rhs; } // Compare an error to an std::error_condition enum. -export auto operator==(error const &lhs, auto rhs) -> bool +export [[nodiscard]] auto operator==(error const &lhs, auto rhs) -> bool requires(std::is_error_condition_enum::value) { return lhs.condition() == rhs; } // Print an error to an ostream. -export auto operator<<(std::ostream &, error const &) -> std::ostream &; +export [[nodiscard]] auto operator<<(std::ostream &, error const &) + -> std::ostream &; } // namespace nihil diff --git a/nihil/exec.cc b/nihil/exec.cc index f29122a..6a7d614 100644 --- a/nihil/exec.cc +++ b/nihil/exec.cc @@ -4,6 +4,8 @@ module; +#include +#include #include #include #include @@ -18,56 +20,47 @@ module nihil; namespace nihil { -exec_error::exec_error(std::string what) - : generic_error(std::move(what)) -{} - -executable_not_found::executable_not_found(std::string_view filename) - : exec_error(std::format("{}: command not found", filename)) - , _filename(filename) -{ -} - -auto executable_not_found::filename(this executable_not_found const &self) --> std::string_view -{ - return self._filename; -} - fexecv::fexecv(fd &&execfd, argv &&args) noexcept - : _execfd(std::move(execfd)) - , _args(std::move(args)) + : m_execfd(std::move(execfd)) + , m_args(std::move(args)) { } -auto fexecv::exec(this fexecv &self) noexcept --> void +auto fexecv::exec(this fexecv &self) + -> std::expected { - ::fexecve(self._execfd.get(), self._args.data(), environ); - ::err(1, "fexecve"); + ::fexecve(self.m_execfd.get(), self.m_args.data(), environ); + return std::unexpected(error("fexecve failed", + error(std::errc(errno)))); } fexecv::fexecv(fexecv &&) noexcept = default; auto fexecv::operator=(this fexecv &, fexecv &&) noexcept -> fexecv& = default; -auto execv(std::string_view path, argv &&argv) -> fexecv +auto execv(std::string_view path, argv &&argv) + -> std::expected { - auto cpath = std::string(path); - auto const ret = ::open(cpath.c_str(), O_EXEC); - if (ret == -1) - throw executable_not_found(path); - return {fd(ret), std::move(argv)}; + auto file = co_await open_file(path, O_EXEC) + .transform_error([&] (error cause) { + return error(std::format("could not open {}", path), + std::move(cause)); + }); + + co_return fexecv(std::move(file), std::move(argv)); } -auto execvp(std::string_view file, argv &&argv) -> fexecv +auto execvp(std::string_view file, argv &&argv) + -> std::expected { auto execfd = find_in_path(file); if (!execfd) - throw executable_not_found(file); - return {std::move(*execfd), std::move(argv)}; + return std::unexpected(error( + std::format("executable not found in path: {}", file))); + return fexecv(std::move(*execfd), std::move(argv)); } -auto shell(std::string_view const &command) -> fexecv +auto shell(std::string_view const &command) + -> std::expected { return execl("/bin/sh", "sh", "-c", command); } diff --git a/nihil/exec.ccm b/nihil/exec.ccm index 7dbf31e..8fcbc14 100644 --- a/nihil/exec.ccm +++ b/nihil/exec.ccm @@ -8,36 +8,19 @@ module; * Exec providers, mostly used for spawn(). */ +#include #include export module nihil:exec; import :argv; +import :error; import :fd; import :find_in_path; import :generic_error; namespace nihil { -/* - * Generic error, what() should be descriptive. - */ -export struct exec_error : generic_error { - exec_error(std::string what); -}; - -/* - * We tried to execute a path or filename and the file was not found. - */ -export struct executable_not_found : exec_error { - executable_not_found(std::string_view filename); - auto filename(this executable_not_found const &self) - -> std::string_view; - -private: - std::string _filename; -}; - /* * A concept to mark spawn executors. */ @@ -47,7 +30,7 @@ export struct exec_tag{}; export template concept executor = requires (T e) { - std::same_as; + std::same_as::tag>; { e.exec() }; }; @@ -63,8 +46,9 @@ export struct fexecv final { fexecv(fd &&execfd, argv &&args) noexcept; - [[noreturn]] auto exec(this fexecv &self) noexcept -> void; - + [[nodiscard]] auto exec(this fexecv &self) + -> std::expected; + // Movable fexecv(fexecv &&) noexcept; auto operator=(this fexecv &, fexecv &&) noexcept -> fexecv&; @@ -74,45 +58,50 @@ export struct fexecv final { auto operator=(this fexecv &, fexecv const &) -> fexecv& = delete; private: - fd _execfd; - argv _args; + fd m_execfd; + argv m_args; }; /* * execv: equivalent to fexecv(), except the command is passed as * a pathname instead of a file descriptor. Does not search $PATH. */ -export auto execv(std::string_view path, argv &&argv) -> fexecv; +export [[nodiscard]] auto execv(std::string_view path, argv &&argv) + -> std::expected; /* * execvp: equivalent to fexecv(), except the command is passed as * a filename instead of a file descriptor. If the filename is not * an absolute path, it will be searched for in $PATH. */ -export auto execvp(std::string_view file, argv &&argv) -> fexecv; +export [[nodiscard]] auto execvp(std::string_view file, argv &&argv) + -> std::expected; /* * execl: equivalent to execv, except the arguments are passed as a * variadic pack of string-like objects. */ -export auto execl(std::string_view path, auto &&...args) -> fexecv +export [[nodiscard]] auto execl(std::string_view path, auto &&...args) + -> std::expected { - return execv(path, argv::from_args({std::string_view(args)...})); + return execv(path, argv({std::string_view(args)...})); } /* * execlp: equivalent to execvp, except the arguments are passed as a * variadic pack of string-like objects. */ -export auto execlp(std::string_view file, auto &&...args) -> fexecv +export [[nodiscard]] auto execlp(std::string_view file, auto &&...args) + -> std::expected { - return execvp(file, argv::from_args({std::string_view(args)...})); + return execvp(file, argv({std::string_view(args)...})); } /* * shell: run the process by invoking /bin/sh -c with the single argument, * equivalent to system(3). */ -export auto shell(std::string_view const &command) -> fexecv; +export [[nodiscard]] auto shell(std::string_view const &command) + -> std::expected; } // namespace nihil diff --git a/nihil/fd.cc b/nihil/fd.cc index 20c3411..3722d2e 100644 --- a/nihil/fd.cc +++ b/nihil/fd.cc @@ -7,6 +7,7 @@ module; #include #include +#include #include #include #include @@ -23,38 +24,38 @@ fd_logic_error::fd_logic_error(std::string what) fd::fd() noexcept = default; -fd::fd(int fd_) noexcept - : _fd(fd_) +fd::fd(int fileno) noexcept + : m_fileno(fileno) { } fd::~fd() { if (*this) - this->close(); + std::ignore = this->close(); } fd::fd(fd &&other) noexcept - : _fd(std::exchange(other._fd, _invalid_fd)) + : m_fileno(std::exchange(other.m_fileno, invalid_fileno)) { } -auto fd::operator=(fd &&other) noexcept -> fd & +auto fd::operator=(this fd &self, fd &&other) noexcept -> fd & { - if (this != &other) - _fd = std::exchange(other._fd, _invalid_fd); - return *this; + if (&self != &other) + self.m_fileno = std::exchange(other.m_fileno, invalid_fileno); + return self; } fd::operator bool(this fd const &self) noexcept { - return self._fd != _invalid_fd; + return self.m_fileno != invalid_fileno; } auto fd::close(this fd &self) -> std::expected { auto const ret = ::close(self.get()); - self._fd = _invalid_fd; + self.m_fileno = invalid_fileno; if (ret == 0) return {}; @@ -65,57 +66,49 @@ auto fd::close(this fd &self) -> std::expected auto fd::get(this fd const &self) -> int { if (self) - return self._fd; + return self.m_fileno; throw fd_logic_error("Attempt to call get() on invalid fd"); } auto fd::release(this fd &&self) -> int { if (self) - return std::exchange(self._fd, self._invalid_fd); + return std::exchange(self.m_fileno, invalid_fileno); throw fd_logic_error("Attempt to release an invalid fd"); } auto dup(fd const &self) -> std::expected { - auto thisfd = self.get(); - - auto const newfd = ::dup(thisfd); + auto const newfd = ::dup(self.get()); if (newfd != -1) - return {newfd}; + return newfd; return std::unexpected(error(std::errc(errno))); } auto dup(fd const &self, int newfd) -> std::expected { - auto thisfd = self.get(); - - auto const ret = ::dup2(thisfd, newfd); + auto const ret = ::dup2(self.get(), newfd); if (ret != -1) - return {newfd}; + return newfd; return std::unexpected(error(std::errc(errno))); } auto raw_dup(fd const &self) -> std::expected { - auto thisfd = self.get(); - - auto const newfd = ::dup(thisfd); + auto const newfd = ::dup(self.get()); if (newfd != -1) - return {newfd}; + return newfd; return std::unexpected(error(std::errc(errno))); } auto raw_dup(fd const &self, int newfd) -> std::expected { - auto thisfd = self.get(); - - auto const ret = ::dup2(thisfd, newfd); + auto const ret = ::dup2(self.get(), newfd); if (ret != -1) - return {newfd}; + return newfd; return std::unexpected(error(std::errc(errno))); } @@ -124,7 +117,7 @@ auto getflags(fd const &self) -> std::expected { auto const flags = ::fcntl(self.get(), F_GETFL); if (flags != -1) - return {flags}; + return flags; return std::unexpected(error(std::errc(errno))); } @@ -140,37 +133,29 @@ auto replaceflags(fd &self, int newflags) -> std::expected auto setflags(fd &self, int newflags) -> std::expected { - auto flags = getflags(self); - if (!flags) - return flags; + auto flags = co_await getflags(self); - *flags |= newflags; - auto const ret = replaceflags(self, *flags); - if (!ret) - return std::unexpected(ret.error()); + flags |= newflags; + co_await replaceflags(self, flags); - return {*flags}; + co_return flags; } auto clearflags(fd &self, int clrflags) -> std::expected { - auto flags = getflags(self); - if (!flags) - return flags; + auto flags = co_await getflags(self); - *flags &= ~clrflags; - auto const ret = replaceflags(self, *flags); - if (!ret) - return std::unexpected(ret.error()); + flags &= ~clrflags; + co_await replaceflags(self, flags); - return {*flags}; + co_return flags; } auto getfdflags(fd const &self) -> std::expected { auto const flags = ::fcntl(self.get(), F_GETFD); if (flags != -1) - return {flags}; + return flags; return std::unexpected(error(std::errc(errno))); } @@ -186,30 +171,22 @@ auto replacefdflags(fd &self, int newflags) -> std::expected auto setfdflags(fd &self, int newflags) -> std::expected { - auto flags = getfdflags(self); - if (!flags) - return flags; + auto flags = co_await getfdflags(self); - *flags |= newflags; - auto const ret = replacefdflags(self, *flags); - if (!ret) - return std::unexpected(ret.error()); + flags |= newflags; + co_await replacefdflags(self, flags); - return {*flags}; + co_return flags; } auto clearfdflags(fd &self, int clrflags) -> std::expected { - auto flags = getfdflags(self); - if (!flags) - return flags; + auto flags = co_await getfdflags(self); - *flags &= ~clrflags; - auto ret = replacefdflags(self, *flags); - if (!ret) - return std::unexpected(ret.error()); + flags &= ~clrflags; + co_await replacefdflags(self, flags); - return {*flags}; + co_return flags; } auto pipe() -> std::expected, error> @@ -233,11 +210,11 @@ auto fd::write(this fd &self, std::span buffer) } auto fd::read(this fd &self, std::span buffer) - -> std::expected + -> std::expected, error> { auto const ret = ::read(self.get(), buffer.data(), buffer.size()); if (ret >= 0) - return ret; + return buffer.subspan(0, ret); return std::unexpected(error(std::errc(errno))); } diff --git a/nihil/fd.ccm b/nihil/fd.ccm index 576f3e8..93a05f5 100644 --- a/nihil/fd.ccm +++ b/nihil/fd.ccm @@ -4,6 +4,7 @@ module; +#include #include #include #include @@ -41,38 +42,38 @@ export struct fd final { // Move from another fd, leaving the moved-from fd in an invalid state. fd(fd &&other) noexcept; - auto operator=(fd &&other) noexcept -> fd &; + auto operator=(this fd &, fd &&other) noexcept -> fd &; // Not copyable. fd(fd const &) = delete; - fd& operator=(fd const &) = delete; + fd& operator=(this fd &, fd const &) = delete; // Return true if this fd is valid (open). - explicit operator bool(this fd const &self) noexcept; + [[nodiscard]] explicit operator bool(this fd const &self) noexcept; // Close the wrapped fd. - auto close(this fd &self) -> std::expected; + [[nodiscard]] auto close(this fd &self) -> std::expected; // Return the stored fd. - auto get(this fd const &self) -> int; + [[nodiscard]] auto get(this fd const &self) -> int; // Release the stored fd and return it. The caller must close it. - auto release(this fd &&self) -> int; + [[nodiscard]] auto release(this fd &&self) -> int; // Write data from the provided buffer to the fd. Returns the // number of bytes written. - auto write(this fd &self, std::span) + [[nodiscard]] auto write(this fd &self, std::span) -> std::expected; - // Read data from the fd to the provided buffer. Returns the - // number of bytes read. - auto read(this fd &self, std::span) - -> std::expected; + // Read data from the fd to the provided buffer. Returns a + // subspan containing the data which was read. + [[nodiscard]] auto read(this fd &self, std::span) + -> std::expected, error>; private: - static constexpr int _invalid_fd = -1; + static constexpr int invalid_fileno = -1; - int _fd = _invalid_fd; + int m_fileno = invalid_fileno; }; // Create a copy of this fd by calling dup(). @@ -125,24 +126,28 @@ export auto pipe() -> std::expected, error>; /* * Write data to a file descriptor from the provided range. Returns the - * number of bytes (not objects) written. Incomplete writes may cause a - * partial object to be written. + * number of bytes written. */ export auto write(fd &file, std::ranges::contiguous_range auto &&range) -> std::expected +requires(sizeof(std::ranges::range_value_t) == 1) { return file.write(as_bytes(std::span(range))); } /* - * Read data from a file descriptor into the provided buffer. Returns the - * number of bytes (not objects) read. Incomplete reads may cause a partial - * object to be read. + * Read data from a file descriptor into the provided buffer. Returns a + * span containing the data that was read. */ export auto read(fd &file, std::ranges::contiguous_range auto &&range) - -> std::expected + -> std::expected< + std::span>, + error> +requires(sizeof(std::ranges::range_value_t) == 1) { - return file.read(as_writable_bytes(std::span(range))); + auto bspan = as_writable_bytes(std::span(range)); + auto rspan = co_await file.read(bspan); + co_return std::span(range).subspan(0, rspan.size()); } } // namespace nihil diff --git a/nihil/getenv.cc b/nihil/getenv.cc index 8f28211..3799d09 100644 --- a/nihil/getenv.cc +++ b/nihil/getenv.cc @@ -17,8 +17,7 @@ module nihil; namespace nihil { - auto getenv(std::string_view varname) --> std::expected +auto getenv(std::string_view varname) -> std::expected { // Start with a buffer of this size, and double it every iteration. constexpr auto bufinc = std::size_t{1024}; @@ -37,7 +36,7 @@ namespace nihil { continue; } - return std::unexpected(std::make_error_code(std::errc(errno))); + return std::unexpected(error(std::errc(errno))); } } diff --git a/nihil/getenv.ccm b/nihil/getenv.ccm index c1b99d7..25bde77 100644 --- a/nihil/getenv.ccm +++ b/nihil/getenv.ccm @@ -10,6 +10,8 @@ module; export module nihil:getenv; +import :error; + namespace nihil { /* @@ -17,6 +19,6 @@ namespace nihil { */ export auto getenv(std::string_view varname) - -> std::expected; + -> std::expected; } // namespace nihil diff --git a/nihil/monad.ccm b/nihil/monad.ccm index 20371c5..9a2db2e 100644 --- a/nihil/monad.ccm +++ b/nihil/monad.ccm @@ -228,9 +228,10 @@ struct expected_promise : expected_promise_base { self.data->emplace(std::move(err)); } - void return_value(this expected_promise &self, int) + void return_value(this expected_promise &self, + std::expected o) { - self.data->emplace(); + self.data->emplace(std::move(o)); } }; diff --git a/nihil/process.cc b/nihil/process.cc index 0122907..5cfe79e 100644 --- a/nihil/process.cc +++ b/nihil/process.cc @@ -6,6 +6,7 @@ module; #include #include +#include #include #include #include @@ -18,29 +19,6 @@ module nihil; namespace nihil { -process_error::process_error(std::string what) - : generic_error(std::move(what)) -{ -} - -waitpid_error::waitpid_error(::pid_t pid, std::error_code error) - : process_error(std::format("waitpid({}): {}", - pid, error.message())) - , _pid(pid) - , _error(error) -{ -} - -auto waitpid_error::pid(this waitpid_error const &self) -> ::pid_t -{ - return self._pid; -} - -auto waitpid_error::error(this waitpid_error const &self) -> std::error_code -{ - return self._error; -} - auto wait_result::okay(this wait_result const &self) -> bool { return self.status() == 0; @@ -70,41 +48,52 @@ wait_result::wait_result(int status) {} process::process(::pid_t pid) - : _pid(pid) + : m_pid(pid) {} process::~process() { - if (_pid == -1) + if (m_pid == -1) return; auto status = int{}; - std::ignore = waitpid(_pid, &status, WEXITED); + std::ignore = waitpid(m_pid, &status, WEXITED); } -process::process(process &&) noexcept = default; -auto process::operator=(process &&) noexcept -> process& = default; +process::process(process &&other) noexcept + : m_pid(std::exchange(other.m_pid, -1)) +{ +} + +auto process::operator=(this process &self, process &&other) noexcept + -> process & +{ + if (&self != &other) { + self.m_pid = std::exchange(other.m_pid, -1); + } + + return self; +} // Get the child's process id. auto process::pid(this process const &self) noexcept -> ::pid_t { - return self._pid; + return self.m_pid; } -auto process::wait(this process &&self) -> wait_result +auto process::wait(this process &&self) -> std::expected { auto status = int{}; - auto ret = waitpid(self._pid, &status, WEXITED); - if (ret != -1) - return wait_result(status); + auto ret = waitpid(self.m_pid, &status, WEXITED); + if (ret == -1) + return std::unexpected(error(std::errc(errno))); - throw waitpid_error(self._pid, - std::make_error_code(std::errc(errno))); + return wait_result(status); } auto process::release(this process &&self) -> ::pid_t { auto const ret = self.pid(); - self._pid = -1; + self.m_pid = -1; return ret; } diff --git a/nihil/process.ccm b/nihil/process.ccm index db8b61a..83e34b4 100644 --- a/nihil/process.ccm +++ b/nihil/process.ccm @@ -4,6 +4,7 @@ module; +#include #include #include #include @@ -12,29 +13,10 @@ module; export module nihil:process; -import :generic_error; +import :error; namespace nihil { -/* - * Exception thrown when a process operation fails. - */ -export struct process_error : generic_error { - process_error(std::string what); -}; - -// A waitpid() call failed. -export struct waitpid_error : process_error { - waitpid_error(::pid_t pid, std::error_code error); - - auto pid(this waitpid_error const &self) -> ::pid_t; - auto error(this waitpid_error const &self) -> std::error_code; - -private: - ::pid_t _pid; - std::error_code _error; -}; - /* * wait_result: the exit status of a process. */ @@ -77,11 +59,11 @@ export struct process final { // Movable. process(process &&) noexcept; - auto operator=(process &&) noexcept -> process&; + auto operator=(this process &, process &&) noexcept -> process &; // Not copyable. process(process const &) = delete; - auto operator=(process const &) -> process& = delete; + auto operator=(this process &, process const &) -> process & = delete; // Get the child's process id. auto pid(this process const &self) noexcept -> ::pid_t; @@ -91,7 +73,7 @@ export struct process final { * its exit status. This destroys the process state, leaving this * object in a moved-from state. */ - auto wait(this process &&self) -> wait_result; + auto wait(this process &&self) -> std::expected; /* * Release this process so we won't try to wait for it when @@ -100,7 +82,7 @@ export struct process final { auto release(this process &&self) -> ::pid_t; private: - ::pid_t _pid; + ::pid_t m_pid; }; } // namespace nihil diff --git a/nihil/read_file.ccm b/nihil/read_file.ccm index 5f332fd..481bf70 100644 --- a/nihil/read_file.ccm +++ b/nihil/read_file.ccm @@ -18,33 +18,28 @@ export module nihil:read_file; import :error; import :fd; +import :monad; import :open_file; namespace nihil { /* - * Load the contents of a file into an output iterator. + * Read the contents of a file into an output iterator. */ -export auto read_file(std::filesystem::path const &filename, - std::output_iterator auto &&iter) +export [[nodiscard]] auto +read_file(std::filesystem::path const &filename, + std::output_iterator auto &&iter) -> std::expected { - auto do_write = [&](fd &&file) -> std::expected - { - auto constexpr bufsize = std::size_t{1024}; - auto buffer = std::array{}; - - for (;;) { - auto err = read(file, buffer); - if (!err) - return std::unexpected(err.error()); - - auto data = std::span(buffer).subspan(0, *err); - std::ranges::copy(data, iter); - } - }; - - return open_file(filename, O_RDONLY).and_then(do_write); + auto file = co_await open_file(filename, O_RDONLY); + + auto constexpr bufsize = std::size_t{1024}; + auto buffer = std::array{}; + + for (;;) { + auto read_buf = co_await(read(file, buffer)); + std::ranges::copy(read_buf, iter); + } } } // namespace nihil diff --git a/nihil/spawn.ccm b/nihil/spawn.ccm index 1ff5776..00c970c 100644 --- a/nihil/spawn.ccm +++ b/nihil/spawn.ccm @@ -10,8 +10,12 @@ module; #include #include +#include +#include +#include #include #include +#include #include #include @@ -26,6 +30,8 @@ export module nihil:spawn; import :argv; import :exec; import :fd; +import :monad; +import :open_file; import :process; namespace nihil { @@ -36,132 +42,137 @@ export inline int constexpr stdout_fileno = STDOUT_FILENO; export inline int constexpr stderr_fileno = STDERR_FILENO; /* - * fd_pipe: create a pipe for the given fd. The first end of the pipe will - * be returned in the given fd object, and the second will be set to the - * given fd number in the child. + * fd_pipe: create a pipe with one end in the child and the other in the + * parent. The child's side will be dup2()'d to the provided fd number. + * The parent side fd can be retrieved via parent_fd(); */ export struct fd_pipe final { - fd_pipe(int fdno, fd &ret) : _fdno(fdno), _fd(&ret) { - auto fds = pipe(); - if (!fds) - throw exec_error(std::format("pipe: {}", - fds.error().what())); - std::tie(_parent_fd, _child_fd) = std::move(*fds); + fd_pipe(int fdno, fd &&child_fd, fd &&parent_fd) + : m_fdno(fdno) + , m_child_fd(std::move(child_fd)) + , m_parent_fd(std::move(parent_fd)) + { } - auto run_in_child(process &) -> void { - _parent_fd.close(); + auto run_in_child(this fd_pipe &self, process &) -> void + { + std::ignore = self.m_parent_fd.close(); - ::dup2(_child_fd.get(), _fdno); - _child_fd.close(); + raw_dup(self.m_child_fd, self.m_fdno); + std::ignore = self.m_child_fd.close(); } - auto run_in_parent(process &) -> void { - _child_fd.close(); - *_fd = std::move(_parent_fd); + auto run_in_parent(this fd_pipe &self, process &) -> void + { + std::ignore = self.m_child_fd.close(); + } + + auto parent_fd(this fd_pipe &self) -> fd & + { + return self.m_parent_fd; } private: - int _fdno; - fd *_fd; - fd _parent_fd; - fd _child_fd; + int m_fdno; + fd m_child_fd; + fd m_parent_fd; }; +export [[nodiscard]] auto +make_fd_pipe(int fdno) -> std::expected +{ + auto fds = co_await pipe(); + co_return fd_pipe(fdno, std::move(fds.first), std::move(fds.second)); +} + /* - * fd_file: open the given file in the child as the given fd. + * fd_file: open a file and provide it to the child as a file descriptor. * open_flags and open_mode are as for ::open(). */ export struct fd_file final { - fd_file(int fdno, - std::string path, - int open_flags, - int open_mode = 0777) - : _fdno(fdno) - , _path(std::move(path)) - , _open_flags(open_flags) - , _open_mode(open_mode) - { } - - auto run_in_parent(process &) -> void {} - - auto run_in_child(process &) -> void { - int newfd = ::open(_path.c_str(), _open_flags, _open_mode); - if (newfd != -1) { - ::dup2(newfd, _fdno); - ::close(newfd); - } + fd_file(int fdno, fd &&file_fd) + : m_fdno(fdno) + , m_file_fd(std::move(file_fd)) + { + } + + auto run_in_parent(this fd_file &self, process &) -> void + { + std::ignore = self.m_file_fd.close(); + } + + auto run_in_child(this fd_file &self, process &) -> void + { + raw_dup(self.m_file_fd, self.m_fdno); + std::ignore = self.m_file_fd.close(); } private: - int _fdno; - std::string _path; - int _open_flags; - int _open_mode; + int m_fdno; + fd m_file_fd; }; +export [[nodiscard]] auto +make_fd_file(int fdno, std::filesystem::path const &file, + int flags, int mode = 0777) + -> std::expected +{ + auto fd = co_await open_file(file, flags, mode); + co_return fd_file(fdno, std::move(fd)); +} + /* * Shorthand for fd_file with /dev/null as the file. */ -export inline auto stdin_devnull() -> fd_file +export [[nodiscard]] inline auto +stdin_devnull() -> std::expected { - return {STDIN_FILENO, "/dev/null", O_RDONLY, 0777}; + return make_fd_file(stdin_fileno, "/dev/null", O_RDONLY); } -export inline auto stdout_devnull() -> fd_file +export [[nodiscard]] inline auto +stdout_devnull() -> std::expected { - return {STDOUT_FILENO, "/dev/null", O_WRONLY, 0777}; + return make_fd_file(stdout_fileno, "/dev/null", O_WRONLY); } -export inline auto stderr_devnull() -> fd_file +export [[nodiscard]] inline auto +stderr_devnull() -> std::expected { - return {STDERR_FILENO, "/dev/null", O_WRONLY, 0777}; + return make_fd_file(stderr_fileno, "/dev/null", O_WRONLY); } /* - * Capture the output of a given file descriptor and write it to the given + * Capture the output of a pipe in the parent and read it into an * output iterator. */ export template Iterator> -struct capture final { - capture(int fdno, Iterator it) - : _fdno(fdno) - , _iterator(std::move(it)) +struct fd_capture final { + fd_capture(fd_pipe &&pipe, Iterator it) + : m_pipe(std::move(pipe)) + , m_iterator(std::move(it)) { - auto fds = pipe(); - if (!fds) - throw exec_error(std::format("pipe: {}", - fds.error().what())); - - std::tie(_parent_fd, _child_fd) = std::move(*fds); } - capture(int fdno, std::string &str) - : capture(fdno, std::back_inserter(str)) - {} - - auto run_in_child(process &) -> void + auto run_in_child(this fd_capture &self, process &p) -> void { - _parent_fd.close(); - - ::dup2(_child_fd.get(), _fdno); - _child_fd.close(); + self.m_pipe.run_in_child(p); } - auto run_in_parent(process &) -> void + auto run_in_parent(this fd_capture &self, process &p) -> void { - _child_fd.close(); + self.m_pipe.run_in_parent(p); - constexpr std::size_t bufsize = 1024; - std::array buffer; - auto ret = ssize_t{}; + auto constexpr bufsize = std::size_t{1024}; + auto buffer = std::array(); + auto &fd = self.m_pipe.parent_fd(); + for (;;) { + auto ret = read(fd, buffer); + if (!ret || ret->size() == 0) + break; - while ((ret = ::read(_parent_fd.get(), - buffer.data(), - buffer.size())) > 0) { - auto data = std::span(buffer).subspan(0, ret); - std::ranges::copy(data, _iterator); + std::ranges::copy(*ret, self.m_iterator); } // We probably want to handle errors here somehow, @@ -169,24 +180,39 @@ struct capture final { } private: - int _fdno; - fd *_fd; - fd _parent_fd; - fd _child_fd; - Iterator _iterator; + fd_pipe m_pipe; + Iterator m_iterator; }; -capture(int, std::string) -> capture>; +export [[nodiscard]] auto +make_capture(int fdno, std::output_iterator auto &&it) + -> std::expected, error> +{ + auto pipe = co_await make_fd_pipe(fdno); + co_return fd_capture(std::move(pipe), + std::forward(it)); +} + +export [[nodiscard]] auto +make_capture(int fdno, std::string &str) + -> std::expected, error> +{ + auto pipe = co_await make_fd_pipe(fdno); + co_return fd_capture(std::move(pipe), std::back_inserter(str)); +} /* * Spawn a new process with the given arguments and return a struct process. * Throws exec_error() on failure. */ -export auto spawn(executor auto &&executor, auto &&...actions) -> process +export [[nodiscard]] auto +spawn(executor auto &&executor, auto &&...actions) + -> std::expected { auto const pid = ::fork(); if (pid == -1) - throw exec_error(std::format("fork: {}", std::strerror(errno))); + return std::unexpected(error("fork failed", + error(std::errc(errno)))); auto proc = process(pid); @@ -198,7 +224,8 @@ export auto spawn(executor auto &&executor, auto &&...actions) -> process std::move(proc).release(); (actions.run_in_child(proc), ...); - executor.exec(); + auto err = executor.exec(); + std::print("{}\n", err.error()); _exit(1); } diff --git a/nihil/tests/fd.cc b/nihil/tests/fd.cc index fbf353e..d8acd35 100644 --- a/nihil/tests/fd.cc +++ b/nihil/tests/fd.cc @@ -191,10 +191,7 @@ TEST_CASE("fd: pipe, read, write", "[fd]") { REQUIRE(*ret == test_string.size()); auto readbuf = std::array{}; - ret = read(fd2, readbuf); - REQUIRE(ret); - REQUIRE(*ret == test_string.size()); - - auto read_string = std::string_view(std::span(readbuf).subspan(0, *ret)); - REQUIRE(read_string == test_string); + auto read_buf = read(fd2, readbuf); + REQUIRE(read_buf); + REQUIRE(std::string_view(*read_buf) == test_string); } diff --git a/nihil/tests/spawn.cc b/nihil/tests/spawn.cc index 455223e..2821fb6 100644 --- a/nihil/tests/spawn.cc +++ b/nihil/tests/spawn.cc @@ -6,60 +6,112 @@ import nihil; -TEST_CASE("spawn: system", "[spawn]") { +TEST_CASE("spawn: system", "[spawn]") +{ using namespace nihil; + + auto exec = shell("x=1; echo $x"); + REQUIRE(exec); + auto output = std::string(); - auto result = spawn(shell("x=1; echo $x"), - capture(stdout_fileno, output)).wait(); + auto capture = make_capture(stdout_fileno, output); + REQUIRE(capture); + + auto proc = spawn(*exec, *capture); + REQUIRE(proc); - REQUIRE(result.okay()); + auto status = std::move(*proc).wait(); + REQUIRE(status); + + REQUIRE(status->okay()); REQUIRE(output == "1\n"); } TEST_CASE("spawn: execv", "[spawn]") { using namespace nihil; + + auto args = argv({"sh", "-c", "x=1; echo $x"}); + auto exec = execv("/bin/sh", std::move(args)); + REQUIRE(exec); + auto output = std::string(); - auto args = argv::from_args({"sh", "-c", "x=1; echo $x"}); - auto result = spawn(execv("/bin/sh", std::move(args)), - capture(stdout_fileno, output)).wait(); + auto capture = make_capture(stdout_fileno, output); + REQUIRE(capture); - REQUIRE(result.okay()); + auto proc = spawn(*exec, *capture); + REQUIRE(proc); + + auto status = std::move(*proc).wait(); + REQUIRE(status); + + REQUIRE(status->okay()); REQUIRE(output == "1\n"); } TEST_CASE("spawn: execvp", "[spawn]") { using namespace nihil; + + auto args = argv({"sh", "-c", "x=1; echo $x"}); + auto exec = execvp("sh", std::move(args)); + REQUIRE(exec); + auto output = std::string(); - auto args = argv::from_args({"sh", "-c", "x=1; echo $x"}); - auto result = spawn(execvp("sh", std::move(args)), - capture(stdout_fileno, output)).wait(); + auto capture = make_capture(stdout_fileno, output); + REQUIRE(capture); + + auto proc = spawn(*exec, *capture); + REQUIRE(proc); - REQUIRE(result.okay()); + auto status = std::move(*proc).wait(); + REQUIRE(status); + + REQUIRE(status->okay()); REQUIRE(output == "1\n"); } TEST_CASE("spawn: execl", "[spawn]") { using namespace nihil; + + auto exec = execl("/bin/sh", "sh", "-c", "x=1; echo $x"); + REQUIRE(exec); + auto output = std::string(); - auto result = spawn(execl("/bin/sh", "sh", "-c", "x=1; echo $x"), - capture(stdout_fileno, output)).wait(); + auto capture = make_capture(stdout_fileno, output); + REQUIRE(capture); + + auto proc = spawn(*exec, *capture); + REQUIRE(proc); + + auto status = std::move(*proc).wait(); + REQUIRE(status); - REQUIRE(result.okay()); + REQUIRE(status->okay()); REQUIRE(output == "1\n"); } TEST_CASE("spawn: execlp", "[spawn]") { using namespace nihil; + + auto exec = execlp("sh", "sh", "-c", "x=1; echo $x"); + REQUIRE(exec); + auto output = std::string(); - auto result = spawn(execlp("sh", "sh", "-c", "x=1; echo $x"), - capture(stdout_fileno, output)).wait(); + auto capture = make_capture(stdout_fileno, output); + REQUIRE(capture); + + auto proc = spawn(*exec, *capture); + REQUIRE(proc); - REQUIRE(result.okay()); + auto status = std::move(*proc).wait(); + REQUIRE(status); + + REQUIRE(status->okay()); REQUIRE(output == "1\n"); } TEST_CASE("spawn: execlp failure", "[spawn]") { using namespace nihil; - REQUIRE_THROWS_AS(execlp("lfjail_nonesuch_executable", "x"), - executable_not_found); + + auto exec = execlp("nihil_no_such_executable", "x"); + REQUIRE(!exec); } -- cgit v1.2.3