From 6d41088153b4b932b4f2524d4252993a5642f998 Mon Sep 17 00:00:00 2001 From: FearlessTobi Date: Mon, 1 Aug 2022 22:47:39 +0200 Subject: [PATCH] core, yuzu: Address first part of review comments --- src/core/announce_multiplayer_session.cpp | 6 +- src/core/hle/service/nifm/nifm.cpp | 1 + src/core/internal_network/network.cpp | 8 +- src/core/internal_network/socket_proxy.cpp | 10 +-- src/core/internal_network/socket_proxy.h | 3 +- src/core/internal_network/sockets.h | 6 +- src/dedicated_room/yuzu-room.cpp | 100 ++++++++++----------- src/yuzu/multiplayer/validation.h | 2 +- src/yuzu/uisettings.h | 5 +- 9 files changed, 70 insertions(+), 71 deletions(-) diff --git a/src/core/announce_multiplayer_session.cpp b/src/core/announce_multiplayer_session.cpp index d73a488cf..6737ce85a 100644 --- a/src/core/announce_multiplayer_session.cpp +++ b/src/core/announce_multiplayer_session.cpp @@ -31,7 +31,7 @@ AnnounceMultiplayerSession::AnnounceMultiplayerSession(Network::RoomNetwork& roo } WebService::WebResult AnnounceMultiplayerSession::Register() { - std::shared_ptr room = room_network.GetRoom().lock(); + auto room = room_network.GetRoom().lock(); if (!room) { return WebService::WebResult{WebService::WebResult::Code::LibError, "Network is not initialized", ""}; @@ -102,7 +102,7 @@ void AnnounceMultiplayerSession::UpdateBackendData(std::shared_ptr lock(callback_mutex); + std::lock_guard lock(callback_mutex); for (auto callback : error_callbacks) { (*callback)(result); } @@ -120,7 +120,7 @@ void AnnounceMultiplayerSession::AnnounceMultiplayerLoop() { std::future future; while (!shutdown_event.WaitUntil(update_time)) { update_time += announce_time_interval; - std::shared_ptr room = room_network.GetRoom().lock(); + auto room = room_network.GetRoom().lock(); if (!room) { break; } diff --git a/src/core/hle/service/nifm/nifm.cpp b/src/core/hle/service/nifm/nifm.cpp index 42ed17187..e3ef06481 100644 --- a/src/core/hle/service/nifm/nifm.cpp +++ b/src/core/hle/service/nifm/nifm.cpp @@ -381,6 +381,7 @@ void IGeneralService::GetCurrentIpAddress(Kernel::HLERequestContext& ctx) { rb.Push(ResultSuccess); rb.PushRaw(*ipv4); } + void IGeneralService::CreateTemporaryNetworkProfile(Kernel::HLERequestContext& ctx) { LOG_DEBUG(Service_NIFM, "called"); diff --git a/src/core/internal_network/network.cpp b/src/core/internal_network/network.cpp index 160cc83e4..3b6906f7d 100644 --- a/src/core/internal_network/network.cpp +++ b/src/core/internal_network/network.cpp @@ -352,8 +352,8 @@ std::optional GetHostIPv4Address() { return {}; } - char ip_addr[16] = {}; - ASSERT(inet_ntop(AF_INET, &interface->ip_address, ip_addr, sizeof(ip_addr)) != nullptr); + std::array ip_addr = {}; + ASSERT(inet_ntop(AF_INET, &interface->ip_address, ip_addr.data(), sizeof(ip_addr)) != nullptr); return TranslateIPv4(interface->ip_address); } @@ -402,9 +402,9 @@ Socket::Socket(Socket&& rhs) noexcept { } template -Errno Socket::SetSockOpt(SOCKET _fd, int option, T value) { +Errno Socket::SetSockOpt(SOCKET fd_, int option, T value) { const int result = - setsockopt(_fd, SOL_SOCKET, option, reinterpret_cast(&value), sizeof(value)); + setsockopt(fd_, SOL_SOCKET, option, reinterpret_cast(&value), sizeof(value)); if (result != SOCKET_ERROR) { return Errno::SUCCESS; } diff --git a/src/core/internal_network/socket_proxy.cpp b/src/core/internal_network/socket_proxy.cpp index b9c50430e..216893ba1 100644 --- a/src/core/internal_network/socket_proxy.cpp +++ b/src/core/internal_network/socket_proxy.cpp @@ -30,19 +30,19 @@ void ProxySocket::HandleProxyPacket(const ProxyPacket& packet) { closed) { return; } - std::lock_guard guard(packets_mutex); + std::lock_guard guard(packets_mutex); received_packets.push(packet); } template -Errno ProxySocket::SetSockOpt(SOCKET _fd, int option, T value) { +Errno ProxySocket::SetSockOpt(SOCKET fd_, int option, T value) { socket_options[option] = reinterpret_cast(&value); return Errno::SUCCESS; } Errno ProxySocket::Initialize(Domain domain, Type type, Protocol socket_protocol) { protocol = socket_protocol; - socket_options[0x1008] = reinterpret_cast(&type); + SetSockOpt(fd, SO_TYPE, type); return Errno::SUCCESS; } @@ -101,7 +101,7 @@ std::pair ProxySocket::RecvFrom(int flags, std::vector& message, ASSERT(message.size() < static_cast(std::numeric_limits::max())); { - std::lock_guard guard(packets_mutex); + std::lock_guard guard(packets_mutex); if (received_packets.size() > 0) { return ReceivePacket(flags, message, addr, message.size()); } @@ -115,7 +115,7 @@ std::pair ProxySocket::RecvFrom(int flags, std::vector& message, return {-1, Errno::AGAIN}; } - std::lock_guard guard(packets_mutex); + std::lock_guard guard(packets_mutex); if (received_packets.size() > 0) { return ReceivePacket(flags, message, addr, message.size()); } diff --git a/src/core/internal_network/socket_proxy.h b/src/core/internal_network/socket_proxy.h index c9155f1af..ad917cac3 100644 --- a/src/core/internal_network/socket_proxy.h +++ b/src/core/internal_network/socket_proxy.h @@ -14,7 +14,7 @@ namespace Network { class ProxySocket : public SocketBase { public: - ProxySocket(RoomNetwork& room_network_) noexcept; + explicit ProxySocket(RoomNetwork& room_network_) noexcept; ~ProxySocket() override; ProxySocket(const ProxySocket&) = delete; @@ -82,6 +82,7 @@ public: bool IsOpened() const override; +private: bool broadcast = false; bool closed = false; u32 send_timeout = 0; diff --git a/src/core/internal_network/sockets.h b/src/core/internal_network/sockets.h index 92dc49993..a70429b19 100644 --- a/src/core/internal_network/sockets.h +++ b/src/core/internal_network/sockets.h @@ -32,7 +32,7 @@ public: std::unique_ptr socket; SockAddrIn sockaddr_in; }; - virtual ~SocketBase() {} + virtual ~SocketBase() = default; virtual SocketBase& operator=(const SocketBase&) = delete; @@ -89,11 +89,7 @@ public: virtual void HandleProxyPacket(const ProxyPacket& packet) = 0; -#if defined(_WIN32) SOCKET fd = INVALID_SOCKET; -#elif YUZU_UNIX - int fd = -1; -#endif }; class Socket : public SocketBase { diff --git a/src/dedicated_room/yuzu-room.cpp b/src/dedicated_room/yuzu-room.cpp index 88645dba7..482e772fb 100644 --- a/src/dedicated_room/yuzu-room.cpp +++ b/src/dedicated_room/yuzu-room.cpp @@ -44,28 +44,30 @@ #endif static void PrintHelp(const char* argv0) { - std::cout << "Usage: " << argv0 - << " [options] \n" - "--room-name The name of the room\n" - "--room-description The room description\n" - "--port The port used for the room\n" - "--max_members The maximum number of players for this room\n" - "--password The password for the room\n" - "--preferred-game The preferred game for this room\n" - "--preferred-game-id The preferred game-id for this room\n" - "--username The username used for announce\n" - "--token The token used for announce\n" - "--web-api-url yuzu Web API url\n" - "--ban-list-file The file for storing the room ban list\n" - "--log-file The file for storing the room log\n" - "--enable-yuzu-mods Allow yuzu Community Moderators to moderate on your room\n" - "-h, --help Display this help and exit\n" - "-v, --version Output version information and exit\n"; + LOG_INFO(Network, + "Usage: {}" + " [options] \n" + "--room-name The name of the room\n" + "--room-description The room description\n" + "--port The port used for the room\n" + "--max_members The maximum number of players for this room\n" + "--password The password for the room\n" + "--preferred-game The preferred game for this room\n" + "--preferred-game-id The preferred game-id for this room\n" + "--username The username used for announce\n" + "--token The token used for announce\n" + "--web-api-url yuzu Web API url\n" + "--ban-list-file The file for storing the room ban list\n" + "--log-file The file for storing the room log\n" + "--enable-yuzu-mods Allow yuzu Community Moderators to moderate on your room\n" + "-h, --help Display this help and exit\n" + "-v, --version Output version information and exit\n", + argv0); } static void PrintVersion() { - std::cout << "yuzu dedicated room " << Common::g_scm_branch << " " << Common::g_scm_desc - << " Libnetwork: " << Network::network_version << std::endl; + LOG_INFO(Network, "yuzu dedicated room {} {} Libnetwork: {}", Common::g_scm_branch, + Common::g_scm_desc, Network::network_version); } /// The magic text at the beginning of a yuzu-room ban list file. @@ -76,7 +78,7 @@ static constexpr char token_delimiter{':'}; static std::string UsernameFromDisplayToken(const std::string& display_token) { std::size_t outlen; - std::array output; + std::array output{}; mbedtls_base64_decode(output.data(), output.size(), &outlen, reinterpret_cast(display_token.c_str()), display_token.length()); @@ -87,7 +89,7 @@ static std::string UsernameFromDisplayToken(const std::string& display_token) { static std::string TokenFromDisplayToken(const std::string& display_token) { std::size_t outlen; - std::array output; + std::array output{}; mbedtls_base64_decode(output.data(), output.size(), &outlen, reinterpret_cast(display_token.c_str()), display_token.length()); @@ -99,13 +101,13 @@ static Network::Room::BanList LoadBanList(const std::string& path) { std::ifstream file; Common::FS::OpenFileStream(file, path, std::ios_base::in); if (!file || file.eof()) { - std::cout << "Could not open ban list!\n\n"; + LOG_ERROR(Network, "Could not open ban list!"); return {}; } std::string magic; std::getline(file, magic); if (magic != BanListMagic) { - std::cout << "Ban list is not valid!\n\n"; + LOG_ERROR(Network, "Ban list is not valid!"); return {}; } @@ -137,7 +139,7 @@ static void SaveBanList(const Network::Room::BanList& ban_list, const std::strin std::ofstream file; Common::FS::OpenFileStream(file, path, std::ios_base::out); if (!file) { - std::cout << "Could not save ban list!\n\n"; + LOG_ERROR(Network, "Could not save ban list!"); return; } @@ -153,8 +155,6 @@ static void SaveBanList(const Network::Room::BanList& ban_list, const std::strin for (const auto& ip : ban_list.second) { file << ip << "\n"; } - - file.flush(); } static void InitializeLogging(const std::string& log_file) { @@ -202,6 +202,8 @@ int main(int argc, char** argv) { {0, 0, 0, 0}, }; + InitializeLogging(log_file); + while (optind < argc) { int arg = getopt_long(argc, argv, "n:d:p:m:w:g:u:t:a:i:l:hv", long_options, &option_index); if (arg != -1) { @@ -256,52 +258,53 @@ int main(int argc, char** argv) { } if (room_name.empty()) { - std::cout << "room name is empty!\n\n"; + LOG_ERROR(Network, "Room name is empty!"); PrintHelp(argv[0]); return -1; } if (preferred_game.empty()) { - std::cout << "preferred game is empty!\n\n"; + LOG_ERROR(Network, "Preferred game is empty!"); PrintHelp(argv[0]); return -1; } if (preferred_game_id == 0) { - std::cout << "preferred-game-id not set!\nThis should get set to allow users to find your " - "room.\nSet with --preferred-game-id id\n\n"; + LOG_ERROR(Network, + "preferred-game-id not set!\nThis should get set to allow users to find your " + "room.\nSet with --preferred-game-id id"); } if (max_members > Network::MaxConcurrentConnections || max_members < 2) { - std::cout << "max_members needs to be in the range 2 - " - << Network::MaxConcurrentConnections << "!\n\n"; + LOG_ERROR(Network, "max_members needs to be in the range 2 - {}!", + Network::MaxConcurrentConnections); PrintHelp(argv[0]); return -1; } - if (port > 65535) { - std::cout << "port needs to be in the range 0 - 65535!\n\n"; + if (port > UINT16_MAX) { + LOG_ERROR(Network, "Port needs to be in the range 0 - 65535!"); PrintHelp(argv[0]); return -1; } if (ban_list_file.empty()) { - std::cout << "Ban list file not set!\nThis should get set to load and save room ban " - "list.\nSet with --ban-list-file \n\n"; + LOG_ERROR(Network, "Ban list file not set!\nThis should get set to load and save room ban " + "list.\nSet with --ban-list-file "); } bool announce = true; if (token.empty() && announce) { announce = false; - std::cout << "token is empty: Hosting a private room\n\n"; + LOG_INFO(Network, "Token is empty: Hosting a private room"); } if (web_api_url.empty() && announce) { announce = false; - std::cout << "endpoint url is empty: Hosting a private room\n\n"; + LOG_INFO(Network, "Endpoint url is empty: Hosting a private room"); } if (announce) { if (username.empty()) { - std::cout << "Hosting a public room\n\n"; + LOG_INFO(Network, "Hosting a public room"); Settings::values.web_api_url = web_api_url; Settings::values.yuzu_username = UsernameFromDisplayToken(token); username = Settings::values.yuzu_username.GetValue(); Settings::values.yuzu_token = TokenFromDisplayToken(token); } else { - std::cout << "Hosting a public room\n\n"; + LOG_INFO(Network, "Hosting a public room"); Settings::values.web_api_url = web_api_url; Settings::values.yuzu_username = username; Settings::values.yuzu_token = token; @@ -309,11 +312,9 @@ int main(int argc, char** argv) { } if (!announce && enable_yuzu_mods) { enable_yuzu_mods = false; - std::cout << "Can not enable yuzu Moderators for private rooms\n\n"; + LOG_INFO(Network, "Can not enable yuzu Moderators for private rooms"); } - InitializeLogging(log_file); - // Load the ban list Network::Room::BanList ban_list; if (!ban_list_file.empty()) { @@ -326,27 +327,26 @@ int main(int argc, char** argv) { verify_backend = std::make_unique(Settings::values.web_api_url.GetValue()); #else - std::cout - << "yuzu Web Services is not available with this build: validation is disabled.\n\n"; + LOG_INFO(Network, + "yuzu Web Services is not available with this build: validation is disabled."); verify_backend = std::make_unique(); #endif } else { verify_backend = std::make_unique(); } - Core::System system{}; - auto& network = system.GetRoomNetwork(); + Network::RoomNetwork network{}; network.Init(); - if (std::shared_ptr room = network.GetRoom().lock()) { + if (auto room = network.GetRoom().lock()) { AnnounceMultiplayerRoom::GameInfo preferred_game_info{.name = preferred_game, .id = preferred_game_id}; if (!room->Create(room_name, room_description, "", port, password, max_members, username, preferred_game_info, std::move(verify_backend), ban_list, enable_yuzu_mods)) { - std::cout << "Failed to create room: \n\n"; + LOG_INFO(Network, "Failed to create room: "); return -1; } - std::cout << "Room is open. Close with Q+Enter...\n\n"; + LOG_INFO(Network, "Room is open. Close with Q+Enter..."); auto announce_session = std::make_unique(network); if (announce) { announce_session->Start(); diff --git a/src/yuzu/multiplayer/validation.h b/src/yuzu/multiplayer/validation.h index 7d48e589d..dabf860be 100644 --- a/src/yuzu/multiplayer/validation.h +++ b/src/yuzu/multiplayer/validation.h @@ -10,7 +10,7 @@ class Validation { public: Validation() - : room_name(room_name_regex), nickname(nickname_regex), ip(ip_regex), port(0, 65535) {} + : room_name(room_name_regex), nickname(nickname_regex), ip(ip_regex), port(0, UINT16_MAX) {} ~Validation() = default; diff --git a/src/yuzu/uisettings.h b/src/yuzu/uisettings.h index 25d1bf1e6..e12d414d9 100644 --- a/src/yuzu/uisettings.h +++ b/src/yuzu/uisettings.h @@ -104,11 +104,12 @@ struct Values { // multiplayer settings Settings::Setting multiplayer_nickname{QStringLiteral("yuzu"), "nickname"}; Settings::Setting multiplayer_ip{{}, "ip"}; - Settings::SwitchableSetting multiplayer_port{24872, 0, 65535, "port"}; + Settings::SwitchableSetting multiplayer_port{24872, 0, UINT16_MAX, "port"}; Settings::Setting multiplayer_room_nickname{{}, "room_nickname"}; Settings::Setting multiplayer_room_name{{}, "room_name"}; Settings::SwitchableSetting multiplayer_max_player{8, 0, 8, "max_player"}; - Settings::SwitchableSetting multiplayer_room_port{24872, 0, 65535, "room_port"}; + Settings::SwitchableSetting multiplayer_room_port{24872, 0, UINT16_MAX, + "room_port"}; Settings::SwitchableSetting multiplayer_host_type{0, 0, 1, "host_type"}; Settings::Setting multiplayer_game_id{{}, "game_id"}; Settings::Setting multiplayer_room_description{{}, "room_description"};