2
\$\begingroup\$

My attempt at an Awaiter for sending scatter-gather data over an open file descriptor. Tested only on gcc-13.1.1

Micro-optimizing by suspending the coroutine only when actually needed. No perf measurements done. I hope to build on this to create a write_all coroutine.

#include <coroutine>
#include <ranges>
#include <concepts>
#include <expected>
#include <span>
#include <string>
#include <cstring>
#include <exception>
#include <iostream>
#include <unistd.h>
#include <sys/socket.h>

template <std::same_as<std::span<char>> ...Args>
class WriteAwaiter{
public:
    WriteAwaiter(int fd, std::span<char>&& arg1, Args&&... args):m_fd(fd){
    // Keeping only "pointers" to the data should be enough, as the awaiting coroutine is responsible for keeping the data valid until it is written
        std::initializer_list<std::span<char>> list{arg1, args...};
        const auto zip = std::ranges::views::zip( list, m_data);
        for (const auto &l : zip ){
            struct iovec &iov = std::get<1>(l);
            auto &data = std::get<0>(l);
            iov.iov_base = data.data();
            iov.iov_len = data.size();
        }
    }
    bool await_ready(){
        return ( m_written = send_int());
    }
    
    std::coroutine_handle<> await_suspend(std::coroutine_handle<> h){
        // HACK to test suspend of awaiter without actually needing the poll() machinery
        return h; // This simulates a single call, resuming h immediately
    }
    
    std::expected<int, int> await_resume(){
        if (!m_written){
            send_int();
        }
        // m_ret must be populated. If we were resumed by POLLOUT, EAGAIN is impossible?
        return m_ret;
    }
    
private:
    
    bool send_int(){
        const struct msghdr msg{
            .msg_name = nullptr,
            .msg_namelen = 0,
            .msg_iov = m_data.data(),
            .msg_iovlen = m_data.size(),
            .msg_control = nullptr,
            .msg_controllen = 0,
            .msg_flags = 0
        };
        int r = sendmsg(m_fd, &msg, MSG_NOSIGNAL);
        if (r < 0 && (errno == EAGAIN || errno == EWOULDBLOCK)){
            return false;
        }
        if (r < 0){
            m_ret = std::unexpected(errno);
        } else {
            m_ret = r;
        }
        
        return true;
    }
    
    bool m_written = false;
    std::expected<int, int> m_ret;
    std::array<struct iovec, sizeof...(Args) + 1> m_data{}; // +1 because one span is mandatory
    int m_fd = -1;
};

template<std::same_as<std::span<char>> ...Args>
WriteAwaiter<Args...> send_aw(int fd, std::span<char> &&arg1, Args&& ...args){
    return WriteAwaiter(fd, std::move(arg1), std::move(args)...);
}

Usage:

class Coroutine {
public:
    struct promise_type{
        Coroutine get_return_object(){ return Coroutine{};}
        std::suspend_never initial_suspend() noexcept {return {};}
        std::suspend_never final_suspend() noexcept { return {};}
        void return_void(){;}
        void unhandled_exception(){
            auto exc = std::current_exception();
            std::rethrow_exception(exc);
        };
    };
};

// minimal coroutine object, gets a connected descriptor
Coroutine run(fd){
    std::string s1("hello");
    std::string s2("world");
    // Example call, missing logic while(*written != s1.size() + s2.size()){ send_aw() ;} ...
    auto written = co_await send_aw(fd, 
                                std::span(s1.begin(), s1.end()), 
                                std::span(s2.begin(), s2.end()));
    if (!written){
        throw std::runtime_error(std::string("Failed to send: ") + std::strerror(written.error()));
    }

int main(){
    int sv[2];
    int r = socketpair(AF_UNIX, SOCK_STREAM, 0, sv);
    if (r != 0){
        std::cout << "socketpair failure " << std::strerror(errno) << std::endl;
        exit(1);
    }
    auto coro = run(sv[1]);
    sleep(10) ; // let coroutine do its thing
}

Ideas welcome on how to allow optional passing of options for msg_control, but that is out of scope for now.

\$\endgroup\$
0

1 Answer 1

3
\$\begingroup\$

Simplify your code

Parts of your code are more verbose than necessary, especially when using newer C++ versions. For example, I would write:

template<std::same_as<std::span<char>> ...Args>
requires (sizeof...(Args) >= 1)
class WriteAwaiter {
public:
    WriteAwaiter(int fd, Args&&... args): m_fd(fd) {
        std::initializer_list list{args...};
        for (auto& [data, iov]: std::ranges::views::zip(list, m_data)) {
            iov = {
                .iov_base = data.data(),
                .iov_len = data.size(),
            };
        }
    }
    …
    std::array<iovec, sizeof...(Args)> m_data;
};


template<typename ...Args>
auto send_aw(int fd, Args&& ...args){
    return WriteAwaiter(fd, std::forward<Args>(args)...);
}

By using the requires clause to check that we have the right number of arguments, I don't need to check it in the constructor, and declaring m_data looks much simpler now, no need for a comment to explain where the + 1 comes from.

Using structured bindings we don't need to call std::get<>(), and since C++20 we can use designated initializers as well.

For send_aw() you don't have to repeat all the constraints you added to WriteAwaiter. Just forward everything as is (using proper perfect forwarding), WriteAwaiter itself will complain if it was not correct. The drawback is that the error message from incorrect use of send_aw() might not be as nice; one solution would be to move the constraints to send_aw() instead, if you don't expect any caller to construct WriteAwaiters directly themselves.

WriteAwaiter's constructor only takes temporaries

Because of the way you wrote the constraint that all Args should be std::span<char>, the code does not work if you pass in an l-value:

std::string str("Hello, world!\n");
std::span arg(str);
write_aw(fd, arg);

This is because Args will then be deduced to std::span<char>&, not std::span<char>. One fix would be to remove the reference from Args before checking it:

template<typename ...Args>
requires (std::same_as<std::remove_reference_t<Args>, std::span<char>> && ...)
…

But even better is to see that it doesn't really matter what the exact type is, as long as you can construct a std::initializer_list<std::span<char>> from the arguments. So as long as it is convertible to a std::span<char> it should be fine:

template<std::convertible_to<std::span<char>> ...Args>
…

This now has the added benefit that you can write:

std::string str("Hello, world!\n");
write_aw(fd, str);

Since a std::string is implicitly convertible to a std::span<char>.

Another solution would be to not make it a template, but rather to take a std::initializer_list<std::span<char>> as a parameter:

class WriteAwaiter {
public:
    WriteAwaiter(int fd, std::initializer_list<std::span<char>> args): m_fd(fd) {
        for (auto& data: args) {
            m_data.push_back(iovec{
                .iov_base = data.data(),
                .iov_len = data.size(),
            });
        }
    }
    …
    std::vector<iovec> m_data;
};

The drawback of that is the need to use braces when calling it, and the fact that its size is not known at compile time.

Allow zero args

As @Deduplicator mentioned, the code still works fine even if sizeof...(Args) == 0. While it is a corner case with little use, it's better to just allow it; nothing will break, and the behavior of sending zero bytes in this case is very reasonable.

Miscellaneous issues

\$\endgroup\$
7
  • \$\begingroup\$ Is there a case for using a std::assignable_to constraint instead of a std::same_as constraint, to fix the problem of passing lvalues? \$\endgroup\$ Jun 6, 2023 at 6:46
  • 1
    \$\begingroup\$ I think you mean std::assignable_from? It is a bit heavyweight since it specifically tests operator=()), which might not be necessary? Perhaps std::convertible_to would be best? \$\endgroup\$
    – G. Sliepen
    Jun 6, 2023 at 8:52
  • \$\begingroup\$ Yes, std::convertible_to sounds better. I never quite know which to use. \$\endgroup\$ Jun 6, 2023 at 9:22
  • 2
    \$\begingroup\$ Disallowing zero is the bane of generic code. Even though zero leads to trivial code, it should still work, maybe even optimized. \$\endgroup\$ Jun 6, 2023 at 21:01
  • 1
    \$\begingroup\$ @serafean Yes, there are always ways to adapt an unfortunate interface, though they may be cumbersome and/or inefficient. If there is a choice though, better fix the interface. Those warts propagate if not explicitly fixed, especially in generic code. \$\endgroup\$ Jun 7, 2023 at 10:17

Your Answer

By clicking “Post Your Answer”, you agree to our terms of service and acknowledge you have read our privacy policy.

Not the answer you're looking for? Browse other questions tagged or ask your own question.