From 5220922a2237cbf868516269f9513a9aaf59558b Mon Sep 17 00:00:00 2001 From: Lioncash Date: Sun, 4 Aug 2019 12:16:16 -0400 Subject: [PATCH 1/6] UICommon/NetPlayIndex: Allow move semantics in SetGame() If the parameter is const, then a move won't actually be able to occur, making the std::move non-functional. We can remove the const qualifier to remedy this. --- Source/Core/UICommon/NetPlayIndex.cpp | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/Source/Core/UICommon/NetPlayIndex.cpp b/Source/Core/UICommon/NetPlayIndex.cpp index 1606b61a76..a55121c32b 100644 --- a/Source/Core/UICommon/NetPlayIndex.cpp +++ b/Source/Core/UICommon/NetPlayIndex.cpp @@ -221,7 +221,7 @@ void NetPlayIndex::SetPlayerCount(int player_count) m_player_count = player_count; } -void NetPlayIndex::SetGame(const std::string game) +void NetPlayIndex::SetGame(std::string game) { m_game = std::move(game); } From 13292563ee43aef52cebc385d82614a60087618b Mon Sep 17 00:00:00 2001 From: Lioncash Date: Sun, 4 Aug 2019 12:19:18 -0400 Subject: [PATCH 2/6] UICommon/NetPlayIndex: Use std::move within SetErrorCallback() std::function is allowed to heap allocate in order to store captured variables, etc, so std::function isn't a trivial type. We can std::move here in order to avoid potential reallocating. While we're at it, make the definition's parameter name match the declaration's parameter name for consistency. --- Source/Core/UICommon/NetPlayIndex.cpp | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-) diff --git a/Source/Core/UICommon/NetPlayIndex.cpp b/Source/Core/UICommon/NetPlayIndex.cpp index a55121c32b..f7cbd3f557 100644 --- a/Source/Core/UICommon/NetPlayIndex.cpp +++ b/Source/Core/UICommon/NetPlayIndex.cpp @@ -333,7 +333,7 @@ bool NetPlayIndex::HasActiveSession() const return !m_secret.empty(); } -void NetPlayIndex::SetErrorCallback(std::function function) +void NetPlayIndex::SetErrorCallback(std::function callback) { - m_error_callback = function; + m_error_callback = std::move(callback); } From 0a67a40e7c8345925b6163d7c287691e7f49e387 Mon Sep 17 00:00:00 2001 From: Lioncash Date: Sun, 4 Aug 2019 12:24:01 -0400 Subject: [PATCH 3/6] UICommon/NetPlayIndex: Move NetPlaySession variable closer to its usage point in List() Moves it closer to where its used, narrowing its visible scope, as well as preventing unnecessary std::string constructor executions in the event invalid data is encountered (the continue branch). --- Source/Core/UICommon/NetPlayIndex.cpp | 3 +-- 1 file changed, 1 insertion(+), 2 deletions(-) diff --git a/Source/Core/UICommon/NetPlayIndex.cpp b/Source/Core/UICommon/NetPlayIndex.cpp index f7cbd3f557..2c24ae41dc 100644 --- a/Source/Core/UICommon/NetPlayIndex.cpp +++ b/Source/Core/UICommon/NetPlayIndex.cpp @@ -86,8 +86,6 @@ NetPlayIndex::List(const std::map& filters) for (const auto& entry : entries.get()) { - NetPlaySession session; - const auto& name = entry.get("name"); const auto& region = entry.get("region"); const auto& method = entry.get("method"); @@ -107,6 +105,7 @@ NetPlayIndex::List(const std::map& filters) continue; } + NetPlaySession session; session.name = name.to_str(); session.region = region.to_str(); session.game_id = game_id.to_str(); From 2830fe820d8eadb333ca6dbdf2f12c902a7f7ed2 Mon Sep 17 00:00:00 2001 From: Lioncash Date: Sun, 4 Aug 2019 12:29:15 -0400 Subject: [PATCH 4/6] UICommon/NetPlayIndex: Take NetPlaySession by const reference for Add() This isn't std::moved wholesale into a member variable or further std::moved into another function, so it's better to take it by const reference here to avoid unnecessary reallocations of contained std::string instances. --- Source/Core/UICommon/NetPlayIndex.cpp | 2 +- Source/Core/UICommon/NetPlayIndex.h | 2 +- 2 files changed, 2 insertions(+), 2 deletions(-) diff --git a/Source/Core/UICommon/NetPlayIndex.cpp b/Source/Core/UICommon/NetPlayIndex.cpp index 2c24ae41dc..c528c8f5e2 100644 --- a/Source/Core/UICommon/NetPlayIndex.cpp +++ b/Source/Core/UICommon/NetPlayIndex.cpp @@ -159,7 +159,7 @@ void NetPlayIndex::NotificationLoop() } } -bool NetPlayIndex::Add(NetPlaySession session) +bool NetPlayIndex::Add(const NetPlaySession& session) { Common::HttpRequest request; auto response = request.Get( diff --git a/Source/Core/UICommon/NetPlayIndex.h b/Source/Core/UICommon/NetPlayIndex.h index f451301152..a94e7daf35 100644 --- a/Source/Core/UICommon/NetPlayIndex.h +++ b/Source/Core/UICommon/NetPlayIndex.h @@ -44,7 +44,7 @@ public: static std::vector> GetRegions(); - bool Add(NetPlaySession session); + bool Add(const NetPlaySession& session); void Remove(); bool HasActiveSession() const; From 75f3656804eb76113fe2e7750f1d03743a039ba6 Mon Sep 17 00:00:00 2001 From: Lioncash Date: Sun, 4 Aug 2019 12:32:29 -0400 Subject: [PATCH 5/6] UICommon/NetPlayIndex: Use a std::string_view for EncryptID()/DecryptID() These parameters only acted as views into the provided strings, so these can just be turned into non-owning string views. --- Source/Core/UICommon/NetPlayIndex.cpp | 4 ++-- Source/Core/UICommon/NetPlayIndex.h | 5 +++-- 2 files changed, 5 insertions(+), 4 deletions(-) diff --git a/Source/Core/UICommon/NetPlayIndex.cpp b/Source/Core/UICommon/NetPlayIndex.cpp index c528c8f5e2..734a739971 100644 --- a/Source/Core/UICommon/NetPlayIndex.cpp +++ b/Source/Core/UICommon/NetPlayIndex.cpp @@ -256,7 +256,7 @@ std::vector> NetPlayIndex::GetRegions() // It isn't very secure but is preferable to adding another dependency on mbedtls // The encrypted data is encoded as nibbles with the character 'A' as the base offset -bool NetPlaySession::EncryptID(const std::string& password) +bool NetPlaySession::EncryptID(std::string_view password) { if (password.empty()) return false; @@ -284,7 +284,7 @@ bool NetPlaySession::EncryptID(const std::string& password) return true; } -std::optional NetPlaySession::DecryptID(const std::string& password) const +std::optional NetPlaySession::DecryptID(std::string_view password) const { if (password.empty()) return {}; diff --git a/Source/Core/UICommon/NetPlayIndex.h b/Source/Core/UICommon/NetPlayIndex.h index a94e7daf35..04f1d8b3b1 100644 --- a/Source/Core/UICommon/NetPlayIndex.h +++ b/Source/Core/UICommon/NetPlayIndex.h @@ -8,6 +8,7 @@ #include #include #include +#include #include #include #include @@ -29,8 +30,8 @@ struct NetPlaySession bool has_password; bool in_game; - bool EncryptID(const std::string& password); - std::optional DecryptID(const std::string& password) const; + bool EncryptID(std::string_view password); + std::optional DecryptID(std::string_view password) const; }; class NetPlayIndex From 8285a94d93370a95c42cb8164928e578964ed6cd Mon Sep 17 00:00:00 2001 From: Lioncash Date: Sun, 4 Aug 2019 12:36:49 -0400 Subject: [PATCH 6/6] UICommon/NetPlayIndex: Take std::vector by const reference in ParseResponse() This variable isn't std::moved anywhere and is just read out of into a string. Instead of making a copy, and then another copy of the data into a std::string, we can take it by reference, only copying the data once. --- Source/Core/UICommon/NetPlayIndex.cpp | 7 ++++--- 1 file changed, 4 insertions(+), 3 deletions(-) diff --git a/Source/Core/UICommon/NetPlayIndex.cpp b/Source/Core/UICommon/NetPlayIndex.cpp index 734a739971..476a43eabc 100644 --- a/Source/Core/UICommon/NetPlayIndex.cpp +++ b/Source/Core/UICommon/NetPlayIndex.cpp @@ -25,13 +25,14 @@ NetPlayIndex::~NetPlayIndex() Remove(); } -static std::optional ParseResponse(std::vector response) +static std::optional ParseResponse(const std::vector& response) { - std::string response_string(reinterpret_cast(response.data()), response.size()); + const std::string response_string(reinterpret_cast(response.data()), + response.size()); picojson::value json; - auto error = picojson::parse(json, response_string); + const auto error = picojson::parse(json, response_string); if (!error.empty()) return {};