Merge pull request #8 from lioncash/bind

Listener: Remove unnecessary bind() within start()

include/jbus/Endpoint.hpp

@@ -1,16 +1,24 @@
 #pragma once
 
-#include "Common.hpp"
+#include <array>
-#include "Socket.hpp"
-#include <thread>
-#include <mutex>
 #include <condition_variable>
+#include <cstddef>
+#include <functional>
+#include <mutex>
+#include <thread>
+
+#include "jbus/Common.hpp"
+#include "jbus/Socket.hpp"
 
 namespace jbus {
 
+using ReadWriteBuffer = std::array<u8, 4>;
+
 /** Main class for performing JoyBoot and subsequent JoyBus I/O operations.
  *  Instances should be obtained though the jbus::Listener::accept method. */
 class Endpoint {
+  using Buffer = std::array<u8, 5>;
+
   /** Self-contained class for solving Kawasedo's GBA BootROM challenge.
    *  GBA will boot client_pad.bin code on completion.
    *
@@ -48,14 +56,14 @@ class Endpoint {
     u32 xc_progLen;
     u8* x10_statusPtr;
     FGBACallback x14_callback;
-    u8 x18_readBuf[4];
+    ReadWriteBuffer x18_readBuf;
-    u8 x1c_writeBuf[4];
+    ReadWriteBuffer x1c_writeBuf;
     s32 x20_byteInWindow;
     u64 x28_ticksAfterXf;
     u32 x30_justStarted;
     u32 x34_bytesSent;
     u32 x38_crc;
-    u32 x3c_checkStore[7];
+    std::array<u32, 7> x3c_checkStore;
     s32 x58_currentKey;
     s32 x5c_initMessage;
     s32 x60_gameId;
@@ -109,7 +117,7 @@ class Endpoint {
   std::condition_variable m_issueCv;
   KawasedoChallenge m_joyBoot;
   FGBACallback m_callback;
-  u8 m_buffer[5];
+  Buffer m_buffer{};
   u8* m_readDstPtr = nullptr;
   u8* m_statusPtr = nullptr;
   u64 m_lastGCTick = 0;
@@ -120,9 +128,9 @@ class Endpoint {
   bool m_running = true;
 
   void clockSync();
-  void send(const u8* buffer);
+  void send(Buffer buffer);
-  size_t receive(u8* buffer);
+  size_t receive(Buffer& buffer);
-  size_t runBuffer(u8* buffer, std::unique_lock<std::mutex>& lk);
+  size_t runBuffer(Buffer& buffer, std::unique_lock<std::mutex>& lk);
   bool idleGetStatus(std::unique_lock<std::mutex>& lk);
   void transferProc();
   void transferWakeup(ThreadLocalEndpoint& endpoint, u8 status);
@@ -163,30 +171,30 @@ public:
   EJoyReturn GBAReset(u8* status);
 
   /** @brief Send READ command to GBA asynchronously.
-   *  @param dst Destination pointer for 4-byte packet of data.
+   *  @param dst Destination reference for 4-byte packet of data.
    *  @param status Destination pointer for EJStatFlags.
    *  @param callback Functor to execute when operation completes.
    *  @return GBA_READY if submitted, or GBA_NOT_READY if another operation in progress. */
-  EJoyReturn GBAReadAsync(u8* dst, u8* status, FGBACallback&& callback);
+  EJoyReturn GBAReadAsync(ReadWriteBuffer& dst, u8* status, FGBACallback&& callback);
 
   /** @brief Send READ command to GBA synchronously.
-   *  @param dst Destination pointer for 4-byte packet of data.
+   *  @param dst Destination reference for 4-byte packet of data.
    *  @param status Destination pointer for EJStatFlags.
    *  @return GBA_READY if submitted, or GBA_NOT_READY if another operation in progress. */
-  EJoyReturn GBARead(u8* dst, u8* status);
+  EJoyReturn GBARead(ReadWriteBuffer& dst, u8* status);
 
   /** @brief Send WRITE command to GBA asynchronously.
    *  @param src Source pointer for 4-byte packet of data. It is not required to keep resident.
    *  @param status Destination pointer for EJStatFlags.
    *  @param callback Functor to execute when operation completes.
    *  @return GBA_READY if submitted, or GBA_NOT_READY if another operation in progress. */
-  EJoyReturn GBAWriteAsync(const u8* src, u8* status, FGBACallback&& callback);
+  EJoyReturn GBAWriteAsync(ReadWriteBuffer src, u8* status, FGBACallback&& callback);
 
   /** @brief Send WRITE command to GBA synchronously.
    *  @param src Source pointer for 4-byte packet of data. It is not required to keep resident.
    *  @param status Destination pointer for EJStatFlags.
    *  @return GBA_READY if submitted, or GBA_NOT_READY if another operation in progress. */
-  EJoyReturn GBAWrite(const u8* src, u8* status);
+  EJoyReturn GBAWrite(ReadWriteBuffer src, u8* status);
 
   /** @brief Initiate JoyBoot sequence on this endpoint.
    *  @param paletteColor Palette for displaying logo in ROM header [0,6].
@@ -241,18 +249,18 @@ public:
   EJoyReturn GBAResetAsync(u8* status, FGBACallback&& callback);
 
   /** @brief Send READ command to GBA asynchronously.
-   *  @param dst Destination pointer for 4-byte packet of data.
+   *  @param dst Destination reference for 4-byte packet of data.
    *  @param status Destination pointer for EJStatFlags.
    *  @param callback Functor to execute when operation completes.
    *  @return GBA_READY if submitted, or GBA_NOT_READY if another operation in progress. */
-  EJoyReturn GBAReadAsync(u8* dst, u8* status, FGBACallback&& callback);
+  EJoyReturn GBAReadAsync(ReadWriteBuffer& dst, u8* status, FGBACallback&& callback);
 
   /** @brief Send WRITE command to GBA asynchronously.
-   *  @param src Source pointer for 4-byte packet of data. It is not required to keep resident.
+   *  @param src 4-byte packet of data. It is not required to keep resident.
    *  @param status Destination pointer for EJStatFlags.
    *  @param callback Functor to execute when operation completes.
    *  @return GBA_READY if submitted, or GBA_NOT_READY if another operation in progress. */
-  EJoyReturn GBAWriteAsync(const u8* src, u8* status, FGBACallback&& callback);
+  EJoyReturn GBAWriteAsync(ReadWriteBuffer src, u8* status, FGBACallback&& callback);
 
   /** @brief Get virtual SI channel assigned to this endpoint.
    *  @return SI channel */

include/jbus/Listener.hpp

@@ -1,12 +1,14 @@
 #pragma once
 
-#include "Common.hpp"
+#include <memory>
-#include "Socket.hpp"
-#include <thread>
-#include <queue>
 #include <mutex>
+#include <queue>
+#include <thread>
+
+#include "jbus/Socket.hpp"
 
 namespace jbus {
+class Endpoint;
 
 /** Server interface for accepting incoming connections from GBA emulator instances. */
 class Listener {
@@ -17,9 +19,6 @@ class Listener {
   std::queue<std::unique_ptr<Endpoint>> m_endpointQueue;
   bool m_running = false;
 
-  static const uint32_t DataPort = 0xd6ba;
-  static const uint32_t ClockPort = 0xc10c;
-
   void listenerProc();
 
 public:
@@ -33,6 +32,7 @@ public:
    *  @return Endpoint instance, ready to issue commands. */
   std::unique_ptr<Endpoint> accept();
 
+  Listener();
   ~Listener();
 };
 

include/jbus/Socket.hpp

@@ -1,17 +1,14 @@
 #pragma once
 
-#include <sys/types.h>
+#include <cstddef>
-#include <fcntl.h>
+#include <cstdint>
 #include <string>
-#include <memory.h>
 
 #ifdef _WIN32
 #include <BaseTsd.h>
-typedef UINT_PTR SOCKET;
+using SOCKET = UINT_PTR;
 #endif
 
-#include "Common.hpp"
-
 struct sockaddr_in;
 
 namespace jbus::net {

lib/Endpoint.cpp

@@ -1,7 +1,13 @@
 #include "jbus/Endpoint.hpp"
 
+#include <algorithm>
+
 #define LOG_TRANSFER 0
 
+#if LOG_TRANSFER
+#include <cstdio>
+#endif
+
 namespace jbus {
 
 #define ROUND_UP_8(val) (((val) + 7) & ~7)
@@ -355,18 +361,20 @@ void Endpoint::clockSync() {
     m_running = false;
 }
 
-void Endpoint::send(const u8* buffer) {
+void Endpoint::send(Buffer buffer) {
   m_lastCmd = buffer[0];
 
   net::Socket::EResult result;
   size_t sentBytes;
-  if (m_lastCmd == CMD_WRITE)
+  if (m_lastCmd == CMD_WRITE) {
-    result = m_dataSocket.send(buffer, 5, sentBytes);
+    result = m_dataSocket.send(buffer.data(), buffer.size(), sentBytes);
-  else
+  } else {
-    result = m_dataSocket.send(buffer, 1, sentBytes);
+    result = m_dataSocket.send(buffer.data(), 1, sentBytes);
+  }
 
-  if (m_lastCmd != CMD_STATUS)
+  if (m_lastCmd != CMD_STATUS) {
     m_booted = true;
+  }
 
   if (result != net::Socket::EResult::OK) {
     m_running = false;
@@ -378,30 +386,31 @@ void Endpoint::send(const u8* buffer) {
 #endif
 }
 
-size_t Endpoint::receive(u8* buffer) {
+size_t Endpoint::receive(Buffer& buffer) {
   if (!m_dataSocket) {
     m_running = false;
-    return 5;
+    return buffer.size();
   }
 
   size_t recvBytes = 0;
-  net::Socket::EResult result = m_dataSocket.recv(buffer, 5, recvBytes);
+  const net::Socket::EResult result = m_dataSocket.recv(buffer.data(), buffer.size(), recvBytes);
   if (result == net::Socket::EResult::Error) {
     m_running = false;
-    return 5;
+    return buffer.size();
   }
 
-  if (recvBytes > 5)
+  if (recvBytes > buffer.size()) {
-    recvBytes = 5;
+    recvBytes = buffer.size();
+  }
 
 #if LOG_TRANSFER
   if (recvBytes > 0) {
     if (m_lastCmd == CMD_STATUS || m_lastCmd == CMD_RESET) {
-      printf("Stat/Reset [< %02x%02x%02x%02x%02x] (%lu)\n", (u8)buffer[0], (u8)buffer[1], (u8)buffer[2], (u8)buffer[3],
+      printf("Stat/Reset [< %02x%02x%02x%02x%02x] (%lu)\n", buffer[0], buffer[1], buffer[2], buffer[3], buffer[4],
-             (u8)buffer[4], recvBytes);
+             recvBytes);
     } else {
-      printf("Receive [< %02x%02x%02x%02x%02x] (%lu)\n", (u8)buffer[0], (u8)buffer[1], (u8)buffer[2], (u8)buffer[3],
+      printf("Receive [< %02x%02x%02x%02x%02x] (%lu)\n", buffer[0], buffer[1], buffer[2], buffer[3], buffer[4],
-             (u8)buffer[4], recvBytes);
+             recvBytes);
     }
   }
 #endif
@@ -409,23 +418,22 @@ size_t Endpoint::receive(u8* buffer) {
   return recvBytes;
 }
 
-size_t Endpoint::runBuffer(u8* buffer, std::unique_lock<std::mutex>& lk) {
+size_t Endpoint::runBuffer(Buffer& buffer, std::unique_lock<std::mutex>& lk) {
-  u8 tmpBuffer[5];
+  Buffer tmpBuffer = buffer;
 
-  memmove(tmpBuffer, buffer, 5);
   lk.unlock();
   clockSync();
   send(tmpBuffer);
-  size_t receivedBytes = receive(tmpBuffer);
+  const size_t receivedBytes = receive(tmpBuffer);
   lk.lock();
-  memmove(buffer, tmpBuffer, 5);
 
+  buffer = tmpBuffer;
   return receivedBytes;
 }
 
 bool Endpoint::idleGetStatus(std::unique_lock<std::mutex>& lk) {
-  u8 buffer[] = {CMD_STATUS, 0, 0, 0, 0};
+  Buffer buffer{u8(CMD_STATUS), 0, 0, 0, 0};
-  return runBuffer(buffer, lk);
+  return runBuffer(buffer, lk) != 0;
 }
 
 void Endpoint::transferProc() {
@@ -455,10 +463,12 @@ void Endpoint::transferProc() {
           *m_statusPtr = m_buffer[0];
         break;
       case CMD_READ:
-        if (m_statusPtr)
+        if (m_statusPtr != nullptr) {
           *m_statusPtr = m_buffer[4];
-        if (m_readDstPtr)
+        }
-          memmove(m_readDstPtr, m_buffer, 4);
+        if (m_readDstPtr != nullptr) {
+          std::copy(m_buffer.cbegin(), m_buffer.cbegin() + 4, m_readDstPtr);
+        }
         break;
       default:
         break;
@@ -594,17 +604,19 @@ EJoyReturn Endpoint::GBAReset(u8* status) {
   return GBA_READY;
 }
 
-EJoyReturn Endpoint::GBAReadAsync(u8* dst, u8* status, FGBACallback&& callback) {
+EJoyReturn Endpoint::GBAReadAsync(ReadWriteBuffer& dst, u8* status, FGBACallback&& callback) {
-  if (!m_running)
+  if (!m_running) {
     return GBA_NOT_READY;
+  }
 
   std::unique_lock<std::mutex> lk(m_syncLock);
-  if (m_cmdIssued)
+  if (m_cmdIssued) {
     return GBA_NOT_READY;
+  }
 
   m_cmdIssued = true;
   m_statusPtr = status;
-  m_readDstPtr = dst;
+  m_readDstPtr = dst.data();
   m_buffer[0] = CMD_READ;
   m_callback = std::move(callback);
 
@@ -613,17 +625,19 @@ EJoyReturn Endpoint::GBAReadAsync(u8* dst, u8* status, FGBACallback&& callback)
   return GBA_READY;
 }
 
-EJoyReturn Endpoint::GBARead(u8* dst, u8* status) {
+EJoyReturn Endpoint::GBARead(ReadWriteBuffer& dst, u8* status) {
-  if (!m_running)
+  if (!m_running) {
     return GBA_NOT_READY;
+  }
 
   std::unique_lock<std::mutex> lk(m_syncLock);
-  if (m_cmdIssued)
+  if (m_cmdIssued) {
     return GBA_NOT_READY;
+  }
 
   m_cmdIssued = true;
   m_statusPtr = status;
-  m_readDstPtr = dst;
+  m_readDstPtr = dst.data();
   m_buffer[0] = CMD_READ;
   m_callback = bindSync();
 
@@ -633,19 +647,22 @@ EJoyReturn Endpoint::GBARead(u8* dst, u8* status) {
   return GBA_READY;
 }
 
-EJoyReturn Endpoint::GBAWriteAsync(const u8* src, u8* status, FGBACallback&& callback) {
+EJoyReturn Endpoint::GBAWriteAsync(ReadWriteBuffer src, u8* status, FGBACallback&& callback) {
-  if (!m_running)
+  if (!m_running) {
     return GBA_NOT_READY;
+  }
 
   std::unique_lock<std::mutex> lk(m_syncLock);
-  if (m_cmdIssued)
+  if (m_cmdIssued) {
     return GBA_NOT_READY;
+  }
 
   m_cmdIssued = true;
   m_statusPtr = status;
   m_buffer[0] = CMD_WRITE;
-  for (int i = 0; i < 4; ++i)
+  for (size_t i = 0; i < src.size(); ++i) {
     m_buffer[i + 1] = src[i];
+  }
   m_callback = std::move(callback);
 
   m_issueCv.notify_one();
@@ -653,19 +670,22 @@ EJoyReturn Endpoint::GBAWriteAsync(const u8* src, u8* status, FGBACallback&& cal
   return GBA_READY;
 }
 
-EJoyReturn Endpoint::GBAWrite(const u8* src, u8* status) {
+EJoyReturn Endpoint::GBAWrite(ReadWriteBuffer src, u8* status) {
-  if (!m_running)
+  if (!m_running) {
     return GBA_NOT_READY;
+  }
 
   std::unique_lock<std::mutex> lk(m_syncLock);
-  if (m_cmdIssued)
+  if (m_cmdIssued) {
     return GBA_NOT_READY;
+  }
 
   m_cmdIssued = true;
   m_statusPtr = status;
   m_buffer[0] = CMD_WRITE;
-  for (int i = 0; i < 4; ++i)
+  for (size_t i = 0; i < src.size(); ++i) {
     m_buffer[i + 1] = src[i];
+  }
   m_callback = bindSync();
 
   m_issueCv.notify_one();
@@ -733,28 +753,29 @@ EJoyReturn ThreadLocalEndpoint::GBAResetAsync(u8* status, FGBACallback&& callbac
   return GBA_READY;
 }
 
-EJoyReturn ThreadLocalEndpoint::GBAReadAsync(u8* dst, u8* status, FGBACallback&& callback) {
+EJoyReturn ThreadLocalEndpoint::GBAReadAsync(ReadWriteBuffer& dst, u8* status, FGBACallback&& callback) {
   if (!m_ep.m_running || m_ep.m_cmdIssued)
     return GBA_NOT_READY;
 
   m_ep.m_cmdIssued = true;
   m_ep.m_statusPtr = status;
-  m_ep.m_readDstPtr = dst;
+  m_ep.m_readDstPtr = dst.data();
   m_ep.m_buffer[0] = Endpoint::CMD_READ;
   m_ep.m_callback = std::move(callback);
 
   return GBA_READY;
 }
 
-EJoyReturn ThreadLocalEndpoint::GBAWriteAsync(const u8* src, u8* status, FGBACallback&& callback) {
+EJoyReturn ThreadLocalEndpoint::GBAWriteAsync(ReadWriteBuffer src, u8* status, FGBACallback&& callback) {
   if (!m_ep.m_running || m_ep.m_cmdIssued)
     return GBA_NOT_READY;
 
   m_ep.m_cmdIssued = true;
   m_ep.m_statusPtr = status;
   m_ep.m_buffer[0] = Endpoint::CMD_WRITE;
-  for (int i = 0; i < 4; ++i)
+  for (size_t i = 0; i < src.size(); ++i) {
     m_ep.m_buffer[i + 1] = src[i];
+  }
   m_ep.m_callback = std::move(callback);
 
   return GBA_READY;

lib/Listener.cpp

@@ -1,9 +1,19 @@
 #include "jbus/Listener.hpp"
+
+#include <cstdint>
+
+#include "jbus/Common.hpp"
 #include "jbus/Endpoint.hpp"
 
 #define LOG_LISTENER 0
 
+#if LOG_LISTENER
+#include <cstdio>
+#endif
+
 namespace jbus {
+constexpr uint32_t DataPort = 0xd6ba;
+constexpr uint32_t ClockPort = 0xc10c;
 
 void Listener::listenerProc() {
 #if LOG_LISTENER
@@ -23,7 +33,7 @@ void Listener::listenerProc() {
         WaitGCTicks(GetGCTicksPerSec());
       } else {
 #if LOG_LISTENER
-        printf("data listening on port %d\n", DataPort);
+        printf("data listening on port %u\n", DataPort);
 #endif
       }
     }
@@ -36,7 +46,7 @@ void Listener::listenerProc() {
         WaitGCTicks(GetGCTicksPerSec());
       } else {
 #if LOG_LISTENER
-        printf("clock listening on port %d\n", ClockPort);
+        printf("clock listening on port %u\n", ClockPort);
 #endif
       }
     }
@@ -58,7 +68,7 @@ void Listener::listenerProc() {
 #endif
     }
     if (acceptData && acceptClock) {
-      std::unique_lock<std::mutex> lk(m_queueLock);
+      std::unique_lock lk{m_queueLock};
       m_endpointQueue.push(std::make_unique<Endpoint>(0, std::move(acceptData), std::move(acceptClock)));
     }
     WaitGCTicks(GetGCTicksPerSec());
@@ -74,7 +84,7 @@ void Listener::listenerProc() {
 void Listener::start() {
   stop();
   m_running = true;
-  m_listenerThread = std::thread(std::bind(&Listener::listenerProc, this));
+  m_listenerThread = std::thread(&Listener::listenerProc, this);
 }
 
 void Listener::stop() {
@@ -84,16 +94,18 @@ void Listener::stop() {
 }
 
 std::unique_ptr<Endpoint> Listener::accept() {
-  std::unique_lock<std::mutex> lk(m_queueLock);
+  std::unique_lock lk{m_queueLock};
-  if (m_endpointQueue.size()) {
+  if (m_endpointQueue.empty()) {
-    std::unique_ptr<Endpoint> ret;
+    return nullptr;
-    ret = std::move(m_endpointQueue.front());
+  }
+
+  auto ret = std::move(m_endpointQueue.front());
   m_endpointQueue.pop();
   return ret;
-  }
-  return {};
 }
 
+Listener::Listener() = default;
+
 Listener::~Listener() { stop(); }
 
 } // namespace jbus

lib/Socket.cpp

@@ -1,13 +1,19 @@
 #include "jbus/Socket.hpp"
 
+#include <cstdio>
+#include <cstring>
+
 #ifndef _WIN32
-#include <sys/socket.h>
+#include <cerrno>
+
+#include <arpa/inet.h>
+#include <fcntl.h>
+#include <netdb.h>
 #include <netinet/in.h>
 #include <netinet/tcp.h>
-#include <arpa/inet.h>
+#include <sys/socket.h>
-#include <netdb.h>
+#include <sys/types.h>
 #include <unistd.h>
-#include <cerrno>
 #else
 #include <WinSock2.h>
 #include <Ws2tcpip.h>
@@ -42,11 +48,10 @@ void IPAddress::resolve(const std::string& address) noexcept {
       m_valid = true;
     } else {
       /* Not a valid address, try to convert it as a host name */
-      addrinfo hints;
+      addrinfo hints = {};
-      memset(&hints, 0, sizeof(hints));
       hints.ai_family = AF_INET;
-      addrinfo* result = NULL;
+      addrinfo* result = nullptr;
-      if (getaddrinfo(address.c_str(), NULL, &hints, &result) == 0) {
+      if (getaddrinfo(address.c_str(), nullptr, &hints, &result) == 0) {
         if (result) {
           addr = reinterpret_cast<sockaddr_in*>(result->ai_addr)->sin_addr;
           freeaddrinfo(result);
@@ -61,8 +66,7 @@ void IPAddress::resolve(const std::string& address) noexcept {
 uint32_t IPAddress::toInteger() const noexcept { return ntohl(m_address); }
 
 static sockaddr_in createAddress(uint32_t address, unsigned short port) {
-  sockaddr_in addr;
+  sockaddr_in addr = {};
-  memset(&addr, 0, sizeof(addr));
   addr.sin_addr.s_addr = htonl(address);
   addr.sin_family = AF_INET;
   addr.sin_port = htons(port);
@@ -102,7 +106,7 @@ void Socket::setRemoteSocket(int remSocket) noexcept {
 }
 
 #ifdef _WIN32
-Socket::EResult Socket::LastWSAError() {
+Socket::EResult Socket::LastWSAError() noexcept {
   switch (WSAGetLastError()) {
   case WSAEWOULDBLOCK:
   case WSAEALREADY: