diff --git a/Source/Core/Core/HW/EXI/BBA/TAPServerBBA.cpp b/Source/Core/Core/HW/EXI/BBA/TAPServerBBA.cpp index 89d8816910..badbbf1ca0 100644 --- a/Source/Core/Core/HW/EXI/BBA/TAPServerBBA.cpp +++ b/Source/Core/Core/HW/EXI/BBA/TAPServerBBA.cpp @@ -3,21 +3,9 @@ #include "Core/HW/EXI/EXI_DeviceEthernet.h" -#ifdef _WIN32 -#include -#include -#else -#include -#include -#include -#include -#include -#endif +#include -#include "Common/CommonFuncs.h" #include "Common/Logging/Log.h" -#include "Common/StringUtil.h" -#include "Core/HW/EXI/EXI_Device.h" namespace ExpansionInterface { @@ -64,7 +52,7 @@ void CEXIETHERNET::TAPServerNetworkInterface::RecvStop() bool CEXIETHERNET::TAPServerNetworkInterface::SendFrame(const u8* frame, u32 size) { - bool ret = m_tapserver_if.SendFrame(frame, size); + const bool ret = m_tapserver_if.SendFrame(frame, size); if (ret) m_eth_ref->SendComplete(); return ret; @@ -76,13 +64,12 @@ void CEXIETHERNET::TAPServerNetworkInterface::HandleReceivedFrame(std::string&& { ERROR_LOG_FMT(SP1, "Received BBA frame of size {}, which is larger than maximum size {}", data.size(), BBA_RECV_SIZE); + return; } - else - { - memcpy(m_eth_ref->mRecvBuffer.get(), data.data(), data.size()); - m_eth_ref->mRecvBufferLength = static_cast(data.size()); - m_eth_ref->RecvHandlePacket(); - } + + std::memcpy(m_eth_ref->mRecvBuffer.get(), data.data(), data.size()); + m_eth_ref->mRecvBufferLength = static_cast(data.size()); + m_eth_ref->RecvHandlePacket(); } } // namespace ExpansionInterface diff --git a/Source/Core/Core/HW/EXI/BBA/TAPServerConnection.cpp b/Source/Core/Core/HW/EXI/BBA/TAPServerConnection.cpp index 4cae9255da..077b306db4 100644 --- a/Source/Core/Core/HW/EXI/BBA/TAPServerConnection.cpp +++ b/Source/Core/Core/HW/EXI/BBA/TAPServerConnection.cpp @@ -14,6 +14,9 @@ #include #endif +#include +#include + #include "Common/CommonFuncs.h" #include "Common/Logging/Log.h" #include "Common/StringUtil.h" @@ -52,7 +55,7 @@ static int ConnectToDestination(const std::string& destination) int ss_size; sockaddr_storage ss; - memset(&ss, 0, sizeof(ss)); + std::memset(&ss, 0, sizeof(ss)); if (destination[0] != '/') { // IP address or hostname @@ -64,10 +67,22 @@ static int ConnectToDestination(const std::string& destination) } sockaddr_in* sin = reinterpret_cast(&ss); - sin->sin_addr.s_addr = htonl(sf::IpAddress(destination.substr(0, colon_offset)).toInteger()); + const sf::IpAddress dest_ip(destination.substr(0, colon_offset)); + if (dest_ip == sf::IpAddress::None || dest_ip == sf::IpAddress::Any) + { + ERROR_LOG_FMT(SP1, "Destination IP address is not valid\n"); + return -1; + } + sin->sin_addr.s_addr = htonl(dest_ip.toInteger()); sin->sin_family = AF_INET; - std::string port_str = destination.substr(colon_offset + 1); - sin->sin_port = htons(atoi(port_str.c_str())); + const std::string port_str = destination.substr(colon_offset + 1); + const int dest_port = std::atoi(port_str.c_str()); + if (dest_port < 1 || dest_port > 65535) + { + ERROR_LOG_FMT(SP1, "Destination port is not valid\n"); + return -1; + } + sin->sin_port = htons(dest_port); ss_size = sizeof(*sin); #ifndef _WIN32 } @@ -81,7 +96,7 @@ static int ConnectToDestination(const std::string& destination) return -1; } sun->sun_family = AF_UNIX; - strcpy(sun->sun_path, destination.c_str()); + std::strcpy(sun->sun_path, destination.c_str()); ss_size = sizeof(*sun); #else } @@ -107,8 +122,8 @@ static int ConnectToDestination(const std::string& destination) if (connect(fd, reinterpret_cast(&ss), ss_size) == -1) { - std::string s = Common::StrNetworkError(); - INFO_LOG_FMT(SP1, "Couldn't connect socket ({}), unable to create tapserver connection\n", s); + INFO_LOG_FMT(SP1, "Couldn't connect socket ({}), unable to create tapserver connection\n", + Common::StrNetworkError()); closesocket(fd); return -1; } @@ -162,31 +177,31 @@ void TAPServerConnection::RecvStop() m_read_enabled.Clear(); } -bool TAPServerConnection::SendAndRemoveAllHDLCFrames(std::string& send_buf) +bool TAPServerConnection::SendAndRemoveAllHDLCFrames(std::string* send_buf) { - while (!send_buf.empty()) + while (!send_buf->empty()) { - std::size_t start_offset = send_buf.find(0x7E); + const std::size_t start_offset = send_buf->find(0x7E); if (start_offset == std::string::npos) { break; } - std::size_t end_sentinel_offset = send_buf.find(0x7E, start_offset + 1); + const std::size_t end_sentinel_offset = send_buf->find(0x7E, start_offset + 1); if (end_sentinel_offset == std::string::npos) { break; } - std::size_t end_offset = end_sentinel_offset + 1; - std::size_t size = end_offset - start_offset; + const std::size_t end_offset = end_sentinel_offset + 1; + const std::size_t size = end_offset - start_offset; - u8 size_bytes[2] = {static_cast(size), static_cast(size >> 8)}; + const u8 size_bytes[2] = {static_cast(size), static_cast(size >> 8)}; if (send(m_fd, reinterpret_cast(size_bytes), 2, SEND_FLAGS) != 2) { ERROR_LOG_FMT(SP1, "SendAndRemoveAllHDLCFrames(): could not write size field"); return false; } const int written_bytes = - send(m_fd, send_buf.data() + start_offset, static_cast(size), SEND_FLAGS); + send(m_fd, send_buf->data() + start_offset, static_cast(size), SEND_FLAGS); if (u32(written_bytes) != size) { ERROR_LOG_FMT(SP1, @@ -194,31 +209,25 @@ bool TAPServerConnection::SendAndRemoveAllHDLCFrames(std::string& send_buf) size, written_bytes); return false; } - else - { - send_buf = send_buf.substr(end_offset); - } + *send_buf = send_buf->substr(end_offset); } return true; } bool TAPServerConnection::SendFrame(const u8* frame, u32 size) { - { - const std::string s = ArrayToString(frame, size, 0x10); - INFO_LOG_FMT(SP1, "SendFrame {}\n{}", size, s); - } + INFO_LOG_FMT(SP1, "SendFrame {}\n{}", size, ArrayToString(frame, size, 0x10)); // On Windows, the data pointer is of type const char*; on other systems it is // of type const void*. This is the reason for the reinterpret_cast here and // in the other send/recv calls in this file. - u8 size_bytes[2] = {static_cast(size), static_cast(size >> 8)}; + const u8 size_bytes[2] = {static_cast(size), static_cast(size >> 8)}; if (send(m_fd, reinterpret_cast(size_bytes), 2, SEND_FLAGS) != 2) { ERROR_LOG_FMT(SP1, "SendFrame(): could not write size field"); return false; } - int written_bytes = + const int written_bytes = send(m_fd, reinterpret_cast(frame), static_cast(size), SEND_FLAGS); if (u32(written_bytes) != size) { @@ -263,7 +272,7 @@ void TAPServerConnection::ReadThreadHandler() case ReadState::SIZE: { u8 size_bytes[2]; - ws_ssize_t bytes_read = recv(m_fd, reinterpret_cast(size_bytes), 2, 0); + const ws_ssize_t bytes_read = recv(m_fd, reinterpret_cast(size_bytes), 2, 0); if (bytes_read == 1) { read_state = ReadState::SIZE_HIGH; @@ -295,25 +304,23 @@ void TAPServerConnection::ReadThreadHandler() // This handles the annoying case where only one byte of the size field // was available earlier. u8 size_high = 0; - ws_ssize_t bytes_read = recv(m_fd, reinterpret_cast(&size_high), 1, 0); - if (bytes_read == 1) - { - frame_bytes_expected |= (size_high << 8); - frame_data.resize(frame_bytes_expected, '\0'); - if (frame_bytes_expected > m_max_frame_size) - { - ERROR_LOG_FMT(SP1, "Packet is too large ({} bytes); dropping it", frame_bytes_expected); - read_state = ReadState::SKIP; - } - else - { - read_state = ReadState::DATA; - } - } - else + const ws_ssize_t bytes_read = recv(m_fd, reinterpret_cast(&size_high), 1, 0); + if (bytes_read != 1) { ERROR_LOG_FMT(SP1, "Failed to read split size field from destination: {}", Common::StrNetworkError()); + break; + } + frame_bytes_expected |= (size_high << 8); + frame_data.resize(frame_bytes_expected, '\0'); + if (frame_bytes_expected > m_max_frame_size) + { + ERROR_LOG_FMT(SP1, "Packet is too large ({} bytes); dropping it", frame_bytes_expected); + read_state = ReadState::SKIP; + } + else + { + read_state = ReadState::DATA; } break; } @@ -321,26 +328,24 @@ void TAPServerConnection::ReadThreadHandler() case ReadState::SKIP: { const std::size_t bytes_to_read = frame_data.size() - frame_bytes_received; - ws_ssize_t bytes_read = recv(m_fd, frame_data.data() + frame_bytes_received, - static_cast(bytes_to_read), 0); + const ws_ssize_t bytes_read = recv(m_fd, frame_data.data() + frame_bytes_received, + static_cast(bytes_to_read), 0); if (bytes_read <= 0) { ERROR_LOG_FMT(SP1, "Failed to read data from destination: {}", Common::StrNetworkError()); + break; } - else + frame_bytes_received += bytes_read; + if (frame_bytes_received == frame_bytes_expected) { - frame_bytes_received += bytes_read; - if (frame_bytes_received == frame_bytes_expected) + if (read_state == ReadState::DATA) { - if (read_state == ReadState::DATA) - { - m_recv_cb(std::move(frame_data)); - } - frame_data.clear(); - frame_bytes_received = 0; - frame_bytes_expected = 0; - read_state = ReadState::SIZE; + m_recv_cb(std::move(frame_data)); } + frame_data.clear(); + frame_bytes_received = 0; + frame_bytes_expected = 0; + read_state = ReadState::SIZE; } break; } diff --git a/Source/Core/Core/HW/EXI/BBA/TAPServerConnection.h b/Source/Core/Core/HW/EXI/BBA/TAPServerConnection.h index ae10fc93a8..f622c312c9 100644 --- a/Source/Core/Core/HW/EXI/BBA/TAPServerConnection.h +++ b/Source/Core/Core/HW/EXI/BBA/TAPServerConnection.h @@ -3,24 +3,12 @@ #pragma once -#ifdef _WIN32 -#include -#include -#else -#include -#include -#include -#include -#include -#endif - #include +#include #include -#include "Common/CommonFuncs.h" -#include "Common/Logging/Log.h" +#include "Common/Flag.h" #include "Common/SocketContext.h" -#include "Common/StringUtil.h" namespace ExpansionInterface { @@ -28,7 +16,9 @@ namespace ExpansionInterface class TAPServerConnection { public: - TAPServerConnection(const std::string& destination, std::function recv_cb, + using RecvCallback = std::function; + + TAPServerConnection(const std::string& destination, RecvCallback recv_cb, std::size_t max_frame_size); bool Activate(); @@ -37,7 +27,7 @@ public: bool RecvInit(); void RecvStart(); void RecvStop(); - bool SendAndRemoveAllHDLCFrames(std::string& send_buf); + bool SendAndRemoveAllHDLCFrames(std::string* send_buf); bool SendFrame(const u8* frame, u32 size); private: @@ -49,9 +39,9 @@ private: Skip, }; - std::string m_destination; - std::function m_recv_cb; - std::size_t m_max_frame_size; + const std::string m_destination; + const std::function m_recv_cb; + const std::size_t m_max_frame_size; Common::SocketContext m_socket_context; int m_fd = -1; diff --git a/Source/Core/Core/HW/EXI/EXI_DeviceModem.cpp b/Source/Core/Core/HW/EXI/EXI_DeviceModem.cpp index 925354c828..02f2e33613 100644 --- a/Source/Core/Core/HW/EXI/EXI_DeviceModem.cpp +++ b/Source/Core/Core/HW/EXI/EXI_DeviceModem.cpp @@ -6,6 +6,7 @@ #include #include +#include #include #include #include @@ -87,12 +88,8 @@ void CEXIModem::ImmWrite(u32 data, u32 size) { // Write device register u8 reg_num = static_cast((m_transfer_descriptor >> 24) & 0x1F); bool should_update_interrupts = false; - for (; size; size--) + for (; size && reg_num < m_regs.size(); size--) { - if (reg_num >= m_regs.size()) - { - break; - } should_update_interrupts |= ((reg_num == Register::INTERRUPT_MASK) || (reg_num == Register::PENDING_INTERRUPT_MASK)); m_regs[reg_num++] = (data >> 24); @@ -153,8 +150,9 @@ u32 CEXIModem::ImmRead(u32 size) return ntohl(be_data); } else - { // Read device register - u8 reg_num = static_cast((m_transfer_descriptor >> 24) & 0x1F); + { + // Read device register + const u8 reg_num = static_cast((m_transfer_descriptor >> 24) & 0x1F); if (reg_num == 0) { return 0x02020000; // Device ID (modem) @@ -214,7 +212,7 @@ void CEXIModem::HandleReadModemTransfer(void* data, u32 size) { // AT command buffer const std::size_t bytes_to_copy = std::min(size, m_at_reply_data.size()); - memcpy(data, m_at_reply_data.data(), bytes_to_copy); + std::memcpy(data, m_at_reply_data.data(), bytes_to_copy); m_at_reply_data = m_at_reply_data.substr(bytes_to_copy); m_regs[Register::AT_REPLY_SIZE] = static_cast(m_at_reply_data.size()); SetInterruptFlag(Interrupt::AT_REPLY_DATA_AVAILABLE, !m_at_reply_data.empty(), true); @@ -224,7 +222,7 @@ void CEXIModem::HandleReadModemTransfer(void* data, u32 size) // Packet receive buffer std::lock_guard g(m_receive_buffer_lock); const std::size_t bytes_to_copy = std::min(size, m_receive_buffer.size()); - memcpy(data, m_receive_buffer.data(), bytes_to_copy); + std::memcpy(data, m_receive_buffer.data(), bytes_to_copy); m_receive_buffer = m_receive_buffer.substr(bytes_to_copy); OnReceiveBufferSizeChangedLocked(true); } @@ -264,7 +262,7 @@ void CEXIModem::HandleWriteModemTransfer(const void* data, u32 size) // from the emulated program's perspective, so we always tell it the send // FIFO is empty. SetInterruptFlag(Interrupt::SEND_BUFFER_BELOW_THRESHOLD, true, true); - m_network_interface->SendAndRemoveAllHDLCFrames(m_send_buffer); + m_network_interface->SendAndRemoveAllHDLCFrames(&m_send_buffer); } else { diff --git a/Source/Core/Core/HW/EXI/EXI_DeviceModem.h b/Source/Core/Core/HW/EXI/EXI_DeviceModem.h index 7d008f869d..786d8db119 100644 --- a/Source/Core/Core/HW/EXI/EXI_DeviceModem.h +++ b/Source/Core/Core/HW/EXI/EXI_DeviceModem.h @@ -17,7 +17,7 @@ class PointerWrap; namespace ExpansionInterface { -#define MODEM_RECV_SIZE 0x800 +static constexpr std::size_t MODEM_RECV_SIZE = 0x800; enum { @@ -44,13 +44,20 @@ public: void DoState(PointerWrap& p) override; private: + // Note: The names in these enums are based on reverse-engineering of PSO and + // not on any documentation of the GC modem or its chipset. If official + // documentation is found, any names in there probably will not match these + // names. + enum Interrupt - { // Used for Register::INTERRUPT_MASK and Register::PENDING_INTERRUPT_MASK + { + // Used for Register::INTERRUPT_MASK and Register::PENDING_INTERRUPT_MASK AT_REPLY_DATA_AVAILABLE = 0x02, SEND_BUFFER_BELOW_THRESHOLD = 0x10, RECEIVE_BUFFER_ABOVE_THRESHOLD = 0x20, RECEIVE_BUFFER_NOT_EMPTY = 0x40, }; + enum Register { DEVICE_TYPE = 0x00, @@ -90,18 +97,22 @@ private: { return transfer_descriptor == 0x80000000; } + static inline bool IsWriteTransfer(u32 transfer_descriptor) { return transfer_descriptor & 0x40000000; } + static inline bool IsModemTransfer(u32 transfer_descriptor) { return transfer_descriptor & 0x20000000; } + static inline u16 GetModemTransferSize(u32 transfer_descriptor) { return (transfer_descriptor >> 8) & 0xFFFF; } + static inline u32 SetModemTransferSize(u32 transfer_descriptor, u16 new_size) { return (transfer_descriptor & 0xFF000000) | (new_size << 8); @@ -117,7 +128,7 @@ private: virtual bool Activate() { return false; } virtual void Deactivate() {} virtual bool IsActivated() { return false; } - virtual bool SendAndRemoveAllHDLCFrames(std::string&) { return false; } + virtual bool SendAndRemoveAllHDLCFrames(std::string*) { return false; } virtual bool RecvInit() { return false; } virtual void RecvStart() {} virtual void RecvStop() {} @@ -134,7 +145,7 @@ private: virtual bool Activate() override; virtual void Deactivate() override; virtual bool IsActivated() override; - virtual bool SendAndRemoveAllHDLCFrames(std::string& send_buffer) override; + virtual bool SendAndRemoveAllHDLCFrames(std::string* send_buffer) override; virtual bool RecvInit() override; virtual void RecvStart() override; virtual void RecvStop() override; diff --git a/Source/Core/Core/HW/EXI/Modem/TAPServerModem.cpp b/Source/Core/Core/HW/EXI/Modem/TAPServerModem.cpp index 4718c34852..5723146b24 100644 --- a/Source/Core/Core/HW/EXI/Modem/TAPServerModem.cpp +++ b/Source/Core/Core/HW/EXI/Modem/TAPServerModem.cpp @@ -3,21 +3,7 @@ #include "Core/HW/EXI/EXI_DeviceModem.h" -#ifdef _WIN32 -#include -#include -#else -#include -#include -#include -#include -#include -#endif - -#include "Common/CommonFuncs.h" #include "Common/Logging/Log.h" -#include "Common/StringUtil.h" -#include "Core/HW/EXI/EXI_Device.h" namespace ExpansionInterface { @@ -47,11 +33,11 @@ bool CEXIModem::TAPServerNetworkInterface::IsActivated() return m_tapserver_if.IsActivated(); } -bool CEXIModem::TAPServerNetworkInterface::SendAndRemoveAllHDLCFrames(std::string& send_buffer) +bool CEXIModem::TAPServerNetworkInterface::SendAndRemoveAllHDLCFrames(std::string* send_buffer) { - std::size_t orig_size = send_buffer.size(); - bool send_succeeded = m_tapserver_if.SendAndRemoveAllHDLCFrames(send_buffer); - if (send_succeeded && (send_buffer.size() < orig_size)) + const std::size_t orig_size = send_buffer->size(); + const bool send_succeeded = m_tapserver_if.SendAndRemoveAllHDLCFrames(send_buffer); + if (send_succeeded && (send_buffer->size() < orig_size)) m_modem_ref->SendComplete(); return send_succeeded; } diff --git a/Source/Core/DolphinQt/Settings/BroadbandAdapterSettingsDialog.cpp b/Source/Core/DolphinQt/Settings/BroadbandAdapterSettingsDialog.cpp index 27579b5ac8..0eecfdf455 100644 --- a/Source/Core/DolphinQt/Settings/BroadbandAdapterSettingsDialog.cpp +++ b/Source/Core/DolphinQt/Settings/BroadbandAdapterSettingsDialog.cpp @@ -51,7 +51,7 @@ void BroadbandAdapterSettingsDialog::InitControls() case Type::TapServer: case Type::ModemTapServer: { - bool is_modem = (m_bba_type == Type::ModemTapServer); + const bool is_modem = (m_bba_type == Type::ModemTapServer); current_address = QString::fromStdString(Config::Get(is_modem ? Config::MAIN_MODEM_TAPSERVER_DESTINATION : Config::MAIN_BBA_TAPSERVER_DESTINATION)); @@ -62,21 +62,13 @@ void BroadbandAdapterSettingsDialog::InitControls() tr("Enter the IP address and port of the tapserver instance you want to connect to.")); #else address_label = new QLabel(tr("Destination (UNIX socket path or address:port):")); - address_placeholder = QStringLiteral("/tmp/dolphin-tap"); - if (is_modem) - { - description = new QLabel( - tr("The default value \"/tmp/dolphin-modem-tap\" will work with a local tapserver and " - "newserv. You " - "can also enter a network location (address:port) to connect to a remote tapserver.")); - } - else - { - description = new QLabel( - tr("The default value \"/tmp/dolphin-tap\" will work with a local tapserver and newserv. " - "You " - "can also enter a network location (address:port) to connect to a remote tapserver.")); - } + address_placeholder = + is_modem ? QStringLiteral(u"/tmp/dolphin-modem-tap") : QStringLiteral(u"/tmp/dolphin-tap"); + description = + new QLabel(tr("The default value \"%1\" will work with a local tapserver and newserv." + " You can also enter a network location (address:port) to connect to a " + "remote tapserver.") + .arg(address_placeholder)); #endif window_title = tr("BBA destination address"); break;