From 5a97e587b63ae62310bb38fc6d1daabe9c9603bc Mon Sep 17 00:00:00 2001 From: Lioncash Date: Tue, 3 Sep 2019 06:25:06 -0400 Subject: [PATCH] Endpoint: Use std::array where applicable Makes the interface a little more strongly-typed, and in some cases makes it less likely to pass in invalid data. --- include/jbus/Endpoint.hpp | 39 ++++++++------ lib/Endpoint.cpp | 109 ++++++++++++++++++++++---------------- 2 files changed, 84 insertions(+), 64 deletions(-) diff --git a/include/jbus/Endpoint.hpp b/include/jbus/Endpoint.hpp index 0c26606..8e33e47 100644 --- a/include/jbus/Endpoint.hpp +++ b/include/jbus/Endpoint.hpp @@ -1,5 +1,6 @@ #pragma once +#include #include #include #include @@ -11,9 +12,13 @@ namespace jbus { +using ReadWriteBuffer = std::array; + /** 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; + /** Self-contained class for solving Kawasedo's GBA BootROM challenge. * GBA will boot client_pad.bin code on completion. * @@ -51,14 +56,14 @@ class Endpoint { u32 xc_progLen; u8* x10_statusPtr; FGBACallback x14_callback; - u8 x18_readBuf[4]; - u8 x1c_writeBuf[4]; + ReadWriteBuffer x18_readBuf; + ReadWriteBuffer x1c_writeBuf; s32 x20_byteInWindow; u64 x28_ticksAfterXf; u32 x30_justStarted; u32 x34_bytesSent; u32 x38_crc; - u32 x3c_checkStore[7]; + std::array x3c_checkStore; s32 x58_currentKey; s32 x5c_initMessage; s32 x60_gameId; @@ -112,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; @@ -123,9 +128,9 @@ class Endpoint { bool m_running = true; void clockSync(); - void send(const u8* buffer); - size_t receive(u8* buffer); - size_t runBuffer(u8* buffer, std::unique_lock& lk); + void send(Buffer buffer); + size_t receive(Buffer& buffer); + size_t runBuffer(Buffer& buffer, std::unique_lock& lk); bool idleGetStatus(std::unique_lock& lk); void transferProc(); void transferWakeup(ThreadLocalEndpoint& endpoint, u8 status); @@ -166,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]. @@ -244,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 */ diff --git a/lib/Endpoint.cpp b/lib/Endpoint.cpp index ad431fe..c825a49 100644 --- a/lib/Endpoint.cpp +++ b/lib/Endpoint.cpp @@ -1,6 +1,6 @@ #include "jbus/Endpoint.hpp" -#include +#include #define LOG_TRANSFER 0 @@ -361,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) - result = m_dataSocket.send(buffer, 5, sentBytes); - else - result = m_dataSocket.send(buffer, 1, sentBytes); + if (m_lastCmd == CMD_WRITE) { + result = m_dataSocket.send(buffer.data(), buffer.size(), sentBytes); + } else { + 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; @@ -384,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) - recvBytes = 5; + if (recvBytes > buffer.size()) { + 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], - (u8)buffer[4], recvBytes); + printf("Stat/Reset [< %02x%02x%02x%02x%02x] (%lu)\n", buffer[0], buffer[1], buffer[2], buffer[3], buffer[4], + recvBytes); } else { - printf("Receive [< %02x%02x%02x%02x%02x] (%lu)\n", (u8)buffer[0], (u8)buffer[1], (u8)buffer[2], (u8)buffer[3], - (u8)buffer[4], recvBytes); + printf("Receive [< %02x%02x%02x%02x%02x] (%lu)\n", buffer[0], buffer[1], buffer[2], buffer[3], buffer[4], + recvBytes); } } #endif @@ -415,23 +418,22 @@ size_t Endpoint::receive(u8* buffer) { return recvBytes; } -size_t Endpoint::runBuffer(u8* buffer, std::unique_lock& lk) { - u8 tmpBuffer[5]; +size_t Endpoint::runBuffer(Buffer& buffer, std::unique_lock& lk) { + 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& lk) { - u8 buffer[] = {CMD_STATUS, 0, 0, 0, 0}; - return runBuffer(buffer, lk); + Buffer buffer{u8(CMD_STATUS), 0, 0, 0, 0}; + return runBuffer(buffer, lk) != 0; } void Endpoint::transferProc() { @@ -461,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; @@ -600,17 +604,19 @@ EJoyReturn Endpoint::GBAReset(u8* status) { return GBA_READY; } -EJoyReturn Endpoint::GBAReadAsync(u8* dst, u8* status, FGBACallback&& callback) { - if (!m_running) +EJoyReturn Endpoint::GBAReadAsync(ReadWriteBuffer& dst, u8* status, FGBACallback&& callback) { + if (!m_running) { return GBA_NOT_READY; + } std::unique_lock 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); @@ -619,17 +625,19 @@ EJoyReturn Endpoint::GBAReadAsync(u8* dst, u8* status, FGBACallback&& callback) return GBA_READY; } -EJoyReturn Endpoint::GBARead(u8* dst, u8* status) { - if (!m_running) +EJoyReturn Endpoint::GBARead(ReadWriteBuffer& dst, u8* status) { + if (!m_running) { return GBA_NOT_READY; + } std::unique_lock 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(); @@ -639,19 +647,22 @@ EJoyReturn Endpoint::GBARead(u8* dst, u8* status) { return GBA_READY; } -EJoyReturn Endpoint::GBAWriteAsync(const u8* src, u8* status, FGBACallback&& callback) { - if (!m_running) +EJoyReturn Endpoint::GBAWriteAsync(ReadWriteBuffer src, u8* status, FGBACallback&& callback) { + if (!m_running) { return GBA_NOT_READY; + } std::unique_lock 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(); @@ -659,19 +670,22 @@ EJoyReturn Endpoint::GBAWriteAsync(const u8* src, u8* status, FGBACallback&& cal return GBA_READY; } -EJoyReturn Endpoint::GBAWrite(const u8* src, u8* status) { - if (!m_running) +EJoyReturn Endpoint::GBAWrite(ReadWriteBuffer src, u8* status) { + if (!m_running) { return GBA_NOT_READY; + } std::unique_lock 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(); @@ -739,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;