Add Unix UDP implementation by vshcryabets · Pull Request #54 · vshcryabets/V2StyxLib

vshcryabets · 2026-02-22T21:13:52Z

No description provided.

Copilot

Pull request overview

This PR factors out Unix socket send logic into a shared transmitter base so TCP can reuse it, and introduces a UDP transmitter type that relies on the same “header + payload in one send” behavior.

Changes:

Removed ChannelUnixTcpTx::sendBuffer() implementation and delegated TCP transmit to a new ChannelUnixSocketTx base.
Added ChannelUnixSocketTx (header + payload combined into a single ::send() call) and wired it into the build.
Added a ChannelUnixUdpTx transmitter type that inherits the shared socket transmitter.

Reviewed changes

Copilot reviewed 6 out of 6 changed files in this pull request and generated 3 comments.

Show a summary per file

File	Description
cpp/modules/l4/src/impl/ChannelUnixTcp.cpp	Removes TCP-specific send implementation (now inherited from shared socket transmitter).
cpp/modules/l4/include/impl/ChannelUnixTcp.h	Makes `ChannelUnixTcpTx` inherit from `ChannelUnixSocketTx` and removes its custom send override.
cpp/modules/l4/include/impl/ChannelUnixSocket.h	Introduces a shared socket-based transmitter API and documentation.
cpp/modules/l4/src/impl/ChannelUnixSocket.cpp	Implements shared socket send behavior (prepends length header, sends combined buffer).
cpp/modules/l4/include/impl/ChannelUnixUdp.h	Adds a UDP transmitter type built on the shared socket transmitter.
cpp/modules/l4/CMakeLists.txt	Adds `ChannelUnixSocket.cpp` to the module build.

💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.

Copilot · 2026-02-22T21:27:30Z

+        Size bytesSent = static_cast<Size>(::send(socket.value(), combined.data(), combined.size(), 0));
+        return bytesSent - headerSize.value();


::send() returns ssize_t and can be -1 on error or less than combined.size() (partial write on TCP). Casting that directly to Size (uint16_t) can overflow/underflow, and bytesSent - headerSize can wrap to a large value. Handle send() errors explicitly (check for <0, map errno as appropriate) and ensure the full header+payload is written (loop until all bytes are sent for TCP) before returning size.

Suggested change

Size bytesSent = static_cast<Size>(::send(socket.value(), combined.data(), combined.size(), 0));

return bytesSent - headerSize.value();

// Ensure the full header + payload is sent. ::send() may write only part of the buffer

// on stream sockets, so we loop until all bytes have been transmitted or an error occurs.

ssize_t totalSent = 0;

const ssize_t totalSize = static_cast<ssize_t>(combined.size());

while (totalSent < totalSize)

{

ssize_t sent = ::send(

socket.value(),

combined.data() + totalSent,

static_cast<size_t>(totalSize - totalSent),

0);

if (sent < 0)

{

// Map any send error to a connection-related error for now.

return std::unexpected(ErrorCode::NotConnected);

}

if (sent == 0)

{

// Peer closed the connection before all data was sent.

return std::unexpected(ErrorCode::NotConnected);

}

totalSent += sent;

}

// At this point, at least the header and some or all of the payload have been sent.

if (totalSent < static_cast<ssize_t>(headerSize.value()))

{

// Should not happen, but guard against underflow when subtracting headerSize.

return std::unexpected(ErrorCode::NotConnected);

}

Size payloadBytesSent = static_cast<Size>(totalSent - static_cast<ssize_t>(headerSize.value()));

return payloadBytesSent;

Copilot · 2026-02-22T21:27:30Z

+         * Header and payload are combined into one ::send() call so that
+         * both TCP and UDP (datagram) transports work correctly.
+         * Returns the number of payload bytes sent without the header 
+         * (i.e. in success case it should return excactly same value as the size parameter), 


Spelling in the doc comment: "excactly" should be "exactly".

Suggested change

* (i.e. in success case it should return excactly same value as the size parameter),

* (i.e. in success case it should return exactly same value as the size parameter),

Copilot · 2026-02-22T21:27:31Z

+        std::vector<uint8_t> combined(headerSize.value() + size);
+        std::memcpy(combined.data(), packetSizeBuffer, headerSize.value());
+        std::memcpy(combined.data() + headerSize.value(), buffer, size);
+        Size bytesSent = static_cast<Size>(::send(socket.value(), combined.data(), combined.size(), 0));


There are existing Catch2 tests for TCP channels, but the new shared ChannelUnixSocketTx::sendBuffer() behavior (header+payload in one send, return value excluding header, error/partial-send handling) isn’t directly covered. Adding focused tests for this method (including a send failure case and verifying the returned payload byte count) would help prevent regressions when refactoring transports (TCP/UDP).

Suggested change

Size bytesSent = static_cast<Size>(::send(socket.value(), combined.data(), combined.size(), 0));

ssize_t rc = ::send(socket.value(), combined.data(), combined.size(), 0);

if (rc < 0)

{

return std::unexpected(ErrorCode::NotConnected);

}

Size bytesSent = static_cast<Size>(rc);

if (bytesSent < headerSize.value())

{

return std::unexpected(ErrorCode::NotConnected);

}

- Introduced `ChannelUnixUdpServer` class inheriting from `ChannelUnixSocketServer` to handle UDP-specific operations. - Implemented methods for creating a UDP server socket, handling incoming datagrams, and sending buffers to clients. - Added packet size handling logic in `getPacketSize` and `sendBuffer` methods. - Enhanced error handling for socket operations and added constants for maximum UDP payload size. - Updated `ChannelUnixSocketServer` to manage client connections and data processing. - Refactored `ChannelUnixTcpServer` to reduce code duplication by reusing common functionalities from `ChannelUnixSocketServer`.

…and tasks

…ility

Copilot

Pull request overview

Copilot reviewed 29 out of 30 changed files in this pull request and generated 16 comments.

💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.

Copilot · 2026-02-23T08:41:31Z

+            configuration.packetSizeHeader,
+            datagram.data(), datagram.size(), size);
+        if (!headerResult)
+            return std::unexpected(headerResult.error());


The function uses std::unexpected but should use the Unexpected wrapper for consistency with the rest of the codebase. All other error returns in this file use Unexpected(ErrorCode::...).

Copilot · 2026-02-23T08:41:32Z

+        // Return the number of payload bytes sent (excluding the header)
+        return static_cast<Size>(sent) - headerBytes;


The sendBuffer function does not verify that sendto() sent all bytes. If sendto() returns a value less than datagram.size(), the subtraction on line 260 will produce an incorrect result. Additionally, if sendto() sends fewer bytes than the header size, the subtraction will cause an integer underflow. The function should check that sent == datagram.size() before returning.

Suggested change

// Return the number of payload bytes sent (excluding the header)

return static_cast<Size>(sent) - headerBytes;

// Ensure the entire datagram (header + payload) was sent.

if (static_cast<size_t>(sent) != datagram.size())

return std::unexpected(ErrorCode::SendFailed);

// Return the number of payload bytes sent (excluding the header)

return size;

Copilot · 2026-02-23T08:41:32Z

+    {
+        for (Socket fd : socketsToClose)
+        {
+            close(fd);


The close() call should use the :: prefix to call the global function consistently with the rest of the codebase. Other parts of the code use ::close() (e.g., lines 73, 87, 94, 203).

Suggested change

close(fd);

::close(fd);

Copilot · 2026-02-23T08:41:32Z

+        fcntl(sock, F_SETFL, flags | O_NONBLOCK);
+


The return value of fcntl() is not checked. If F_GETFL fails, flags will contain -1, and the subsequent F_SETFL call will set invalid flags. Both fcntl calls should check for errors and handle them appropriately.

Suggested change

fcntl(sock, F_SETFL, flags | O_NONBLOCK);

if (flags == -1)

{

::close(sock);

startPromise->set_value(ErrorCode::CantCreateSocket);

return InvalidFileDescriptor;

}

if (fcntl(sock, F_SETFL, flags | O_NONBLOCK) == -1)

{

::close(sock);

startPromise->set_value(ErrorCode::CantCreateSocket);

return InvalidFileDescriptor;

}

Copilot · 2026-02-23T08:41:32Z

+                sockaddr_in serverAddress{};
+                serverAddress.sin_family = AF_INET;
+                serverAddress.sin_port = htons(configuration.port);
+                inet_pton(AF_INET, configuration.address.c_str(), &serverAddress.sin_addr);


The return value of inet_pton() is not checked. If it returns 0 (invalid format) or -1 (error), the connect() will likely fail with an incorrect address, but the error message will not indicate the root cause. The return value should be checked, and if it's not 1 (success), the socket should be closed and an appropriate error returned.

Suggested change

inet_pton(AF_INET, configuration.address.c_str(), &serverAddress.sin_addr);

int ptonResult = ::inet_pton(AF_INET, configuration.address.c_str(), &serverAddress.sin_addr);

if (ptonResult != 1)

{

::close(sock);

return ErrorCode::NotConnected;

}

Copilot · 2026-02-23T08:41:34Z

+        Size size)
+    {
+        if (!isStarted())
+            return std::unexpected(ErrorCode::NotConnected);


The function uses std::unexpected but should use the Unexpected wrapper for consistency with the rest of the codebase. All other error returns in this file use Unexpected(ErrorCode::...).

Copilot · 2026-02-23T08:41:34Z

+
+        auto addrIt = clientIdToSockAddr.find(clientId);
+        if (addrIt == clientIdToSockAddr.end())
+            return std::unexpected(ErrorCode::UnknownClient);


The function uses std::unexpected but should use the Unexpected wrapper for consistency with the rest of the codebase. All other error returns in this file use Unexpected(ErrorCode::...).

Copilot · 2026-02-23T08:41:34Z

        fcntl(serverSocket, F_SETFL, flags | O_NONBLOCK);



The return value of fcntl() is not checked. If F_GETFL fails, flags will contain -1, and the subsequent F_SETFL call will set invalid flags. Both fcntl calls should check for errors and handle them appropriately.

Suggested change

fcntl(serverSocket, F_SETFL, flags | O_NONBLOCK);

if (flags == -1)

{

::close(serverSocket);

startPromise->set_value(ErrorCode::CantCreateSocket);

return InvalidFileDescriptor;

}

if (fcntl(serverSocket, F_SETFL, flags | O_NONBLOCK) == -1)

{

::close(serverSocket);

startPromise->set_value(ErrorCode::CantCreateSocket);

return InvalidFileDescriptor;

}

Copilot · 2026-02-23T08:41:35Z

        }

        int opt = 1;
        if (setsockopt(serverSocket, SOL_SOCKET, SO_REUSEADDR, &opt, sizeof(opt)) < 0)


The setsockopt() call should use the :: prefix to call the global function consistently with the rest of the codebase. Other system calls like ::bind(), ::listen(), ::close() consistently use the :: prefix.

Suggested change

if (setsockopt(serverSocket, SOL_SOCKET, SO_REUSEADDR, &opt, sizeof(opt)) < 0)

if (::setsockopt(serverSocket, SOL_SOCKET, SO_REUSEADDR, &opt, sizeof(opt)) < 0)

Copilot · 2026-02-23T08:41:35Z

+                    ClientId clientId;
+                    auto addrIt = addressToClientId.find(addrKey);
+                    if (addrIt == addressToClientId.end())
+                    {
+                        clientId = configuration.clientsRepo->getNextClientId();
+                        addressToClientId[addrKey] = clientId;
+                        clientIdToSockAddr[clientId] = peerAddr;
+
+                        // Register in the base map (buffer unused) so that
+                        // publishClients() iteration works unchanged.
+                        ClientFullInfo info;
+                        info.id      = clientId;
+                        info.address = ipStr;
+                        info.port    = peerPort;
+                        // No ReadBuffer needed – UDP datagrams go directly to
+                        // the deserializer. isDirty stays false forever so
+                        // processBuffers() is a no-op for UDP entries.
+                        socketToClientInfoMapFull.insert({static_cast<Socket>(clientId), std::move(info)});
+                        publishClients();
+                    }
+                    else
+                    {
+                        clientId = addrIt->second;
+                    }


The addressToClientId and clientIdToSockAddr maps are accessed from both the server thread (in handlePollEvents which adds entries) and potentially from other threads (in sendBuffer which reads entries). These maps are not protected by any mutex, which could lead to data races if sendBuffer is called from a different thread than the server's work thread. The base class's clientIdToChannelClient map has the same issue and may require synchronization.

vshcryabets self-assigned this Feb 22, 2026

vshcryabets requested a review from Copilot February 22, 2026 21:21

Copilot started reviewing on behalf of vshcryabets February 22, 2026 21:21 View session

Copilot AI reviewed Feb 22, 2026

View reviewed changes

vshcryabets added 3 commits February 23, 2026 09:07

Add Unix UDP implementation

6314329

Fix ChannelUnixSocketTx::sendBuffer

ef1c9e6

Add ChannelUnixUdpClient

05a2955

vshcryabets force-pushed the test/udp branch from 822e0bb to d8f3878 Compare February 23, 2026 07:28

vshcryabets added 3 commits February 23, 2026 09:33

Enhance CMake configuration for CPP17 and CPP23 support in workflows …

4b278d6

…and tasks

Refactor Channel classes and update CMake tasks for C++17/23 compatib…

a1dd184

…ility

vshcryabets requested a review from Copilot February 23, 2026 08:35

Copilot started reviewing on behalf of vshcryabets February 23, 2026 08:35 View session

vshcryabets marked this pull request as draft February 23, 2026 08:39

vshcryabets marked this pull request as ready for review February 23, 2026 08:39

Copilot AI reviewed Feb 23, 2026

View reviewed changes

vshcryabets marked this pull request as draft February 23, 2026 08:52

vshcryabets marked this pull request as ready for review February 23, 2026 08:52

Fix comments

3aaa831

vshcryabets merged commit 453c270 into master Feb 23, 2026
3 checks passed

vshcryabets deleted the test/udp branch March 7, 2026 08:28

		Size bytesSent = static_cast<Size>(::send(socket.value(), combined.data(), combined.size(), 0));
		return bytesSent - headerSize.value();

-        Size bytesSent = static_cast<Size>(::send(socket.value(), combined.data(), combined.size(), 0));
-        return bytesSent - headerSize.value();
+        // Ensure the full header + payload is sent. ::send() may write only part of the buffer
+        // on stream sockets, so we loop until all bytes have been transmitted or an error occurs.
+        ssize_t totalSent = 0;
+        const ssize_t totalSize = static_cast<ssize_t>(combined.size());
+        while (totalSent < totalSize)
+        {
+            ssize_t sent = ::send(
+                socket.value(),
+                combined.data() + totalSent,
+                static_cast<size_t>(totalSize - totalSent),
+);
+            if (sent < 0)
+            {
+                // Map any send error to a connection-related error for now.
+                return std::unexpected(ErrorCode::NotConnected);
+            }
+            if (sent == 0)
+            {
+                // Peer closed the connection before all data was sent.
+                return std::unexpected(ErrorCode::NotConnected);
+            }
+            totalSent += sent;
+        }
+        // At this point, at least the header and some or all of the payload have been sent.
+        if (totalSent < static_cast<ssize_t>(headerSize.value()))
+        {
+            // Should not happen, but guard against underflow when subtracting headerSize.
+            return std::unexpected(ErrorCode::NotConnected);
+        }
+        Size payloadBytesSent = static_cast<Size>(totalSent - static_cast<ssize_t>(headerSize.value()));
+        return payloadBytesSent;

	* (i.e. in success case it should return excactly same value as the size parameter),
	* (i.e. in success case it should return exactly same value as the size parameter),

-        Size bytesSent = static_cast<Size>(::send(socket.value(), combined.data(), combined.size(), 0));
+        ssize_t rc = ::send(socket.value(), combined.data(), combined.size(), 0);
+        if (rc < 0)
+        {
+            return std::unexpected(ErrorCode::NotConnected);
+        }
+        Size bytesSent = static_cast<Size>(rc);
+        if (bytesSent < headerSize.value())
+        {
+            return std::unexpected(ErrorCode::NotConnected);
+        }

		// Return the number of payload bytes sent (excluding the header)
		return static_cast<Size>(sent) - headerBytes;

-        // Return the number of payload bytes sent (excluding the header)
-        return static_cast<Size>(sent) - headerBytes;
+        // Ensure the entire datagram (header + payload) was sent.
+        if (static_cast<size_t>(sent) != datagram.size())
+            return std::unexpected(ErrorCode::SendFailed);
+        // Return the number of payload bytes sent (excluding the header)
+        return size;

-        fcntl(sock, F_SETFL, flags | O_NONBLOCK);
+        if (flags == -1)
+        {
+            ::close(sock);
+            startPromise->set_value(ErrorCode::CantCreateSocket);
+            return InvalidFileDescriptor;
+        }
+        if (fcntl(sock, F_SETFL, flags | O_NONBLOCK) == -1)
+        {
+            ::close(sock);
+            startPromise->set_value(ErrorCode::CantCreateSocket);
+            return InvalidFileDescriptor;
+        }

-                inet_pton(AF_INET, configuration.address.c_str(), &serverAddress.sin_addr);
+                int ptonResult = ::inet_pton(AF_INET, configuration.address.c_str(), &serverAddress.sin_addr);
+                if (ptonResult != 1)
+                {
+                    ::close(sock);
+                    return ErrorCode::NotConnected;
+                }

	if (setsockopt(serverSocket, SOL_SOCKET, SO_REUSEADDR, &opt, sizeof(opt)) < 0)
	if (::setsockopt(serverSocket, SOL_SOCKET, SO_REUSEADDR, &opt, sizeof(opt)) < 0)

Conversation

vshcryabets commented Feb 22, 2026

Uh oh!

Copilot AI left a comment

Choose a reason for hiding this comment

Pull request overview

Reviewed changes

Uh oh!

Copilot AI Feb 22, 2026

Choose a reason for hiding this comment

Uh oh!

Copilot AI Feb 22, 2026

Choose a reason for hiding this comment

Uh oh!

Copilot AI Feb 22, 2026

Choose a reason for hiding this comment

Uh oh!

Copilot AI left a comment

Choose a reason for hiding this comment

Pull request overview

Uh oh!

Copilot AI Feb 23, 2026

Choose a reason for hiding this comment

Uh oh!

Copilot AI Feb 23, 2026

Choose a reason for hiding this comment

Uh oh!

Copilot AI Feb 23, 2026

Choose a reason for hiding this comment

Uh oh!

Copilot AI Feb 23, 2026

Choose a reason for hiding this comment

Uh oh!

Copilot AI Feb 23, 2026

Choose a reason for hiding this comment

Uh oh!

Copilot AI Feb 23, 2026

Choose a reason for hiding this comment

Uh oh!

Copilot AI Feb 23, 2026

Choose a reason for hiding this comment

Uh oh!

Copilot AI Feb 23, 2026

Choose a reason for hiding this comment

Uh oh!

Copilot AI Feb 23, 2026

Choose a reason for hiding this comment

Uh oh!

Copilot AI Feb 23, 2026

Choose a reason for hiding this comment

Uh oh!

Uh oh!

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

2 participants