Skip to content

Commit

Permalink
net/SocketDescriptor: add {Read,Write}NoWait()
Browse files Browse the repository at this point in the history
It was surprising that Read() was non-blocking, but there was no
blocking version of it.  Let's make the non-blocking behavior explicit
and change Read() to be blocking.

In order to find existing callers easily with compiler errors, this
also refactors Read()/Write() to take a std::span parameter.
  • Loading branch information
MaxKellermann committed Sep 27, 2023
1 parent cad35a8 commit 491cc8f
Show file tree
Hide file tree
Showing 7 changed files with 47 additions and 18 deletions.
7 changes: 5 additions & 2 deletions src/client/New.cxx
Original file line number Diff line number Diff line change
Expand Up @@ -11,12 +11,15 @@
#include "lib/fmt/SocketAddressFormatter.hxx"
#include "net/UniqueSocketDescriptor.hxx"
#include "net/SocketAddress.hxx"
#include "util/SpanCast.hxx"
#include "Log.hxx"
#include "Version.h"

#include <cassert>

static constexpr char GREETING[] = "OK MPD " PROTOCOL_VERSION "\n";
using std::string_view_literals::operator""sv;

static constexpr auto GREETING = "OK MPD " PROTOCOL_VERSION "\n"sv;

Client::Client(EventLoop &_loop, Partition &_partition,
UniqueSocketDescriptor _fd,
Expand Down Expand Up @@ -49,7 +52,7 @@ client_new(EventLoop &loop, Partition &partition,
return;
}

(void)fd.Write(GREETING, sizeof(GREETING) - 1);
(void)fd.WriteNoWait(AsBytes(GREETING));

const unsigned num = next_client_num++;
auto *client = new Client(loop, partition, std::move(fd), uid,
Expand Down
2 changes: 1 addition & 1 deletion src/event/BufferedSocket.cxx
Original file line number Diff line number Diff line change
Expand Up @@ -9,7 +9,7 @@
inline BufferedSocket::ssize_t
BufferedSocket::DirectRead(std::span<std::byte> dest) noexcept
{
const auto nbytes = GetSocket().Read((char *)dest.data(), dest.size());
const auto nbytes = GetSocket().ReadNoWait(dest);
if (nbytes > 0) [[likely]]
return nbytes;

Expand Down
2 changes: 1 addition & 1 deletion src/event/FullyBufferedSocket.cxx
Original file line number Diff line number Diff line change
Expand Up @@ -11,7 +11,7 @@
inline FullyBufferedSocket::ssize_t
FullyBufferedSocket::DirectWrite(std::span<const std::byte> src) noexcept
{
const auto nbytes = GetSocket().Write((const char *)src.data(), src.size());
const auto nbytes = GetSocket().WriteNoWait(src);
if (nbytes < 0) [[unlikely]] {
const auto code = GetSocketError();
if (IsSocketErrorSendWouldBlock(code))
Expand Down
15 changes: 13 additions & 2 deletions src/net/SocketDescriptor.cxx
Original file line number Diff line number Diff line change
Expand Up @@ -420,14 +420,25 @@ SocketDescriptor::Send(std::span<const std::byte> src, int flags) const noexcept
}

ssize_t
SocketDescriptor::Read(void *buffer, std::size_t length) const noexcept
SocketDescriptor::ReadNoWait(std::span<std::byte> dest) const noexcept
{
int flags = 0;
#ifndef _WIN32
flags |= MSG_DONTWAIT;
#endif

return Receive({static_cast<std::byte *>(buffer), length}, flags);
return Receive(dest, flags);
}

ssize_t
SocketDescriptor::WriteNoWait(std::span<const std::byte> src) const noexcept
{
int flags = 0;
#ifndef _WIN32
flags |= MSG_DONTWAIT;
#endif

return Send(src, flags);
}

#ifdef _WIN32
Expand Down
20 changes: 17 additions & 3 deletions src/net/SocketDescriptor.hxx
Original file line number Diff line number Diff line change
Expand Up @@ -274,12 +274,26 @@ public:
*/
ssize_t Send(std::span<const std::byte> src, int flags=0) const noexcept;

ssize_t Read(void *buffer, std::size_t length) const noexcept;
ssize_t Read(std::span<std::byte> dest) const noexcept {
return Receive(dest);
}

ssize_t Write(const void *buffer, std::size_t length) const noexcept {
return Send({static_cast<const std::byte *>(buffer), length});
ssize_t Write(std::span<const std::byte> src) const noexcept {
return Send(src);
}

/**
* Wrapper for Receive() with MSG_DONTWAIT (not available on
* Windows).
*/
ssize_t ReadNoWait(std::span<std::byte> dest) const noexcept;

/**
* Wrapper for Receive() with MSG_DONTWAIT (not available on
* Windows).
*/
ssize_t WriteNoWait(std::span<const std::byte> src) const noexcept;

#ifdef _WIN32
int WaitReadable(int timeout_ms) const noexcept;
int WaitWritable(int timeout_ms) const noexcept;
Expand Down
12 changes: 6 additions & 6 deletions src/output/plugins/httpd/HttpdClient.cxx
Original file line number Diff line number Diff line change
Expand Up @@ -9,6 +9,7 @@
#include "IcyMetaDataServer.hxx"
#include "net/SocketError.hxx"
#include "net/UniqueSocketDescriptor.hxx"
#include "util/SpanCast.hxx"
#include "Log.hxx"

#include <fmt/core.h>
Expand Down Expand Up @@ -150,7 +151,7 @@ HttpdClient::SendResponse() noexcept
response = allocated.c_str();
}

ssize_t nbytes = GetSocket().Write(response, strlen(response));
ssize_t nbytes = GetSocket().WriteNoWait(AsBytes(std::string_view{response}));
if (nbytes < 0) [[unlikely]] {
const SocketErrorMessage msg;
FmtWarning(httpd_output_domain,
Expand Down Expand Up @@ -207,16 +208,15 @@ HttpdClient::TryWritePage(const Page &page, size_t position) noexcept
{
assert(position < page.size());

return GetSocket().Write(page.data() + position,
page.size() - position);
return GetSocket().WriteNoWait(std::span<const std::byte>{page}.subspan(position));
}

ssize_t
HttpdClient::TryWritePageN(const Page &page,
size_t position, ssize_t n) noexcept
{
return n >= 0
? GetSocket().Write(page.data() + position, n)
? GetSocket().WriteNoWait({page.data() + position, (std::size_t)n})
: TryWritePage(page, position);
}

Expand Down Expand Up @@ -283,9 +283,9 @@ HttpdClient::TryWrite() noexcept
metadata_sent = true;
}
} else {
char empty_data = 0;
static constexpr std::byte empty_data[1]{};

ssize_t nbytes = GetSocket().Write(&empty_data, 1);
ssize_t nbytes = GetSocket().Write(empty_data);
if (nbytes < 0) {
auto e = GetSocketError();
if (IsSocketErrorSendWouldBlock(e))
Expand Down
7 changes: 4 additions & 3 deletions src/system/EventPipe.cxx
Original file line number Diff line number Diff line change
Expand Up @@ -37,8 +37,8 @@ EventPipe::Read() noexcept
assert(r.IsDefined());
assert(w.IsDefined());

char buffer[256];
return r.Read(buffer, sizeof(buffer)) > 0;
std::byte buffer[256];
return r.Read(buffer) > 0;
}

void
Expand All @@ -47,7 +47,8 @@ EventPipe::Write() noexcept
assert(r.IsDefined());
assert(w.IsDefined());

w.Write("", 1);
static constexpr std::byte buffer[1]{};
w.Write(buffer);
}

#ifdef _WIN32
Expand Down

0 comments on commit 491cc8f

Please sign in to comment.