Skip to content

Make readClientRequest fallible #12

New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

Sign up for GitHub

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Open
wants to merge 2 commits into
base: master
Choose a base branch
from
Open

Make readClientRequest fallible #12

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
2 commits
Select commit Hold shift + click to select a range
f74f1f7
Add expect overload for std::optional
RuthgerD May 17, 2025
391ba22
Make readClientRequest fallible + simple read/write wrappers
RuthgerD May 17, 2025
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
3 changes: 1 addition & 2 deletions src/fastipc.cxx
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -37,7 +37,6 @@
#include <sys/mman.h>
#include <sys/socket.h>
#include <sys/un.h>
#include <unistd.h>

#include "io/cursor.hxx"
#include "io/fd.hxx"
Expand Down Expand Up @@ -81,7 +80,7 @@ void writeClientRequest(std::span<std::byte>& buf, const ClientRequest& request)
writeClientRequest(sndbuf, request);

const auto bytes_written =
expect(io::sysVal(::write(sockfd.fd(), buf.data(), buf.size() - sndbuf.size())), "failed to write to tower");
expect(io::write(sockfd, std::span{buf}.first(buf.size() - sndbuf.size())), "failed to write to tower");
static_cast<void>(bytes_written); // seq packet

std::size_t total_size{0U};
Expand Down
17 changes: 15 additions & 2 deletions src/io/fd.hxx
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -18,8 +18,9 @@

#pragma once

#include <cerrno>
#include <cstddef>
#include <expected>
#include <span>
#include <utility>

#include <unistd.h>
Expand Down Expand Up @@ -57,8 +58,20 @@ class Fd final {
int m_fd{-1};
};

[[nodiscard]] constexpr io::expected<Fd> adoptSysFd(int fd) noexcept {
[[nodiscard]] constexpr expected<Fd> adoptSysFd(int fd) noexcept {
return sysVal(fd).transform([](int fd) { return Fd{fd}; });
}

[[nodiscard]] inline expected<std::size_t> write(const Fd& fd, std::span<const std::byte> buf) noexcept {
return sysVal(::write(fd.fd(), buf.data(), buf.size())).transform([](int written) {
return static_cast<std::size_t>(written);
});
}

[[nodiscard]] inline expected<std::size_t> read(const Fd& fd, std::span<std::byte> buf) noexcept {
return sysVal(::read(fd.fd(), buf.data(), buf.size())).transform([](int read) {
return static_cast<std::size_t>(read);
});
}

} // namespace fastipc::io
16 changes: 13 additions & 3 deletions src/io/result.hxx
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -20,7 +20,7 @@

#include <concepts>
#include <expected>
#include <iostream>
#include <print>
#include <system_error>

namespace fastipc {
Expand Down Expand Up @@ -68,7 +68,7 @@ template <typename T>
return std::move(expected.value());
}

std::cerr << message << ": " << expected.error().message() << "\n" << std::flush;
std::println(stderr, "{}: {}", message, expected.error().message());
std::abort();
}

Expand All @@ -77,7 +77,17 @@ inline void expect(std::expected<void, std::error_code> expected, std::string_vi
return;
}

std::cerr << message << ": " << expected.error().message() << "\n" << std::flush;
std::println(stderr, "{}: {}", message, expected.error().message());
std::abort();
}

template <typename T>
[[nodiscard]] T expect(std::optional<T> expected, std::string_view message = "unexpected") noexcept {
if (expected.has_value()) {
return std::move(expected.value());
}

std::println(stderr, "{}", message);
std::abort();
}

Expand Down
38 changes: 29 additions & 9 deletions src/tower.cxx
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -21,12 +21,12 @@
#include <array>
#include <atomic>
#include <cassert>
#include <csignal>
#include <cstddef>
#include <cstdint>
#include <cstdio>
#include <cstring>
#include <limits>
#include <optional>
#include <print>
#include <span>
#include <string>
Expand All @@ -49,19 +49,40 @@
namespace fastipc {
namespace {

[[nodiscard]] ClientRequest readClientRequest(std::span<const std::byte>& buf) noexcept {
[[nodiscard]] constexpr io::expected<std::optional<ClientRequest>>
Copy link
Member

@AeroStun AeroStun May 19, 2025 •
edited
Loading

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Instead of nesting things, can we only use the expected and put std::errc::invalid_argument if the input is garbage?

Copy link
Collaborator Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

the idea is that if the optional is empty then we need more bytes, if there is an unexpected value then the input is garbage. We can make a new error code that says like "give me more bytes" or something to flatten it

readClientRequest(std::span<const std::byte>& obuf) noexcept {
constexpr static auto kMinSize = 10u;

auto buf = obuf;

if (buf.size() < kMinSize) {
return {};
}

const auto requester_type = io::getBuf<std::underlying_type_t<RequesterType>>(buf);

if (requester_type >= 2) {
return io::unexpected{std::make_error_code(std::errc::protocol_error)};
}

const auto max_payload_size = io::getBuf<std::size_t>(buf);
const auto topic_name_buf = io::takeBuf(buf, io::getBuf<std::uint8_t>(buf));
const auto topic_name_size = io::getBuf<std::uint8_t>(buf);

if (buf.size() < topic_name_size) {
return {};
}

assert(requester_type < 2);
const auto topic_name_buf = io::takeBuf(buf, topic_name_size);

return {
obuf = buf;

return ClientRequest{
.type = static_cast<RequesterType>(requester_type),
.max_payload_size = max_payload_size,
.topic_name = {reinterpret_cast<const char*>(topic_name_buf.data()), topic_name_buf.size()},
};
}

} // namespace

[[nodiscard]] Tower Tower::create(std::string_view path) {
Expand Down Expand Up @@ -107,11 +128,10 @@ void Tower::shutdown() { expect(io::sysCheck(::shutdown(m_sockfd.fd(), SHUT_RD))

void Tower::serve(io::Fd clientfd) {
std::array<std::byte, 128u> buf{}; // NOLINT(*-magic-numbers)
const auto bytes_read =
expect(io::sysVal(::read(clientfd.fd(), buf.data(), buf.size())), "failed to read from client");
const auto bytes_read = expect(io::read(clientfd, std::span{buf}), "failed to read from client");

auto recvbuf = std::span<const std::byte>{buf.data(), static_cast<std::size_t>(bytes_read)};
const auto request = readClientRequest(recvbuf);
auto recvbuf = std::span<const std::byte>{buf}.first(bytes_read);
const auto request = expect(expect(readClientRequest(recvbuf), "invalid request"), "incomplete message");

std::println("{} request for topic '{}' with max payload size of {} bytes.",
(request.type == RequesterType::Reader ? "reader" : "writer"), request.topic_name,
Expand Down