Merge pull request #8154 from lioncash/analytics

{Common/Analytics, Core/Analytics}: Minor C++17 transitional changes

Source/Core/Common/Analytics.cpp

@@ -76,15 +76,19 @@ AnalyticsReportBuilder::AnalyticsReportBuilder()
   m_report.push_back(WIRE_FORMAT_VERSION);
 }
 
-void AnalyticsReportBuilder::AppendSerializedValue(std::string* report, const std::string& v)
+void AnalyticsReportBuilder::AppendSerializedValue(std::string* report, std::string_view v)
 {
   AppendType(report, TypeId::STRING);
   AppendBytes(report, reinterpret_cast<const u8*>(v.data()), static_cast<u32>(v.size()));
 }
 
+// We can't remove this overload despite the string_view overload due to the fact that
+// pointers can implicitly convert to bool, so if we removed the overload, then all
+// const char strings passed in would begin forwarding to the bool overload,
+// which is definitely not what we want to occur.
 void AnalyticsReportBuilder::AppendSerializedValue(std::string* report, const char* v)
 {
-  AppendSerializedValue(report, std::string(v));
+  AppendSerializedValue(report, std::string_view(v));
 }
 
 void AnalyticsReportBuilder::AppendSerializedValue(std::string* report, bool v)
@@ -198,7 +202,7 @@ void StdoutAnalyticsBackend::Send(std::string report)
          HexDump(reinterpret_cast<const u8*>(report.data()), report.size()).c_str());
 }
 
-HttpAnalyticsBackend::HttpAnalyticsBackend(const std::string& endpoint) : m_endpoint(endpoint)
+HttpAnalyticsBackend::HttpAnalyticsBackend(std::string endpoint) : m_endpoint(std::move(endpoint))
 {
 }
 

Source/Core/Common/Analytics.h

@@ -8,6 +8,7 @@
 #include <memory>
 #include <mutex>
 #include <string>
+#include <string_view>
 #include <thread>
 #include <utility>
 #include <vector>
@@ -46,7 +47,7 @@ namespace Common
 class AnalyticsReportingBackend
 {
 public:
-  virtual ~AnalyticsReportingBackend() {}
+  virtual ~AnalyticsReportingBackend() = default;
   // Called from the AnalyticsReporter backend thread.
   virtual void Send(std::string report) = 0;
 };
@@ -61,7 +62,7 @@ public:
   AnalyticsReportBuilder(const AnalyticsReportBuilder& other) { *this = other; }
   AnalyticsReportBuilder(AnalyticsReportBuilder&& other)
   {
-    std::lock_guard<std::mutex> lk(other.m_lock);
+    std::lock_guard lk{other.m_lock};
     m_report = std::move(other.m_report);
   }
 
@@ -69,8 +70,7 @@ public:
   {
     if (this != &other)
     {
-      std::lock_guard<std::mutex> lk(m_lock);
+      std::scoped_lock lk{m_lock, other.m_lock};
-      std::lock_guard<std::mutex> lk2(other.m_lock);
       m_report = other.m_report;
     }
     return *this;
@@ -81,24 +81,24 @@ public:
   {
     // Get before locking the object to avoid deadlocks with this += this.
     std::string other_report = other.Get();
-    std::lock_guard<std::mutex> lk(m_lock);
+    std::lock_guard lk{m_lock};
     m_report += other_report;
     return *this;
   }
 
   template <typename T>
-  AnalyticsReportBuilder& AddData(const std::string& key, const T& value)
+  AnalyticsReportBuilder& AddData(std::string_view key, const T& value)
   {
-    std::lock_guard<std::mutex> lk(m_lock);
+    std::lock_guard lk{m_lock};
     AppendSerializedValue(&m_report, key);
     AppendSerializedValue(&m_report, value);
     return *this;
   }
 
   template <typename T>
-  AnalyticsReportBuilder& AddData(const std::string& key, const std::vector<T>& value)
+  AnalyticsReportBuilder& AddData(std::string_view key, const std::vector<T>& value)
   {
-    std::lock_guard<std::mutex> lk(m_lock);
+    std::lock_guard lk{m_lock};
     AppendSerializedValue(&m_report, key);
     AppendSerializedValueVector(&m_report, value);
     return *this;
@@ -106,19 +106,19 @@ public:
 
   std::string Get() const
   {
-    std::lock_guard<std::mutex> lk(m_lock);
+    std::lock_guard lk{m_lock};
     return m_report;
   }
 
   // More efficient version of Get().
   std::string Consume()
   {
-    std::lock_guard<std::mutex> lk(m_lock);
+    std::lock_guard lk{m_lock};
     return std::move(m_report);
   }
 
 protected:
-  static void AppendSerializedValue(std::string* report, const std::string& v);
+  static void AppendSerializedValue(std::string* report, std::string_view v);
   static void AppendSerializedValue(std::string* report, const char* v);
   static void AppendSerializedValue(std::string* report, bool v);
   static void AppendSerializedValue(std::string* report, u64 v);
@@ -185,7 +185,7 @@ public:
 class HttpAnalyticsBackend : public AnalyticsReportingBackend
 {
 public:
-  HttpAnalyticsBackend(const std::string& endpoint);
+  explicit HttpAnalyticsBackend(std::string endpoint);
   ~HttpAnalyticsBackend() override;
 
   void Send(std::string report) override;

Source/Core/Core/Analytics.cpp

@@ -1,5 +1,6 @@
 #include "Core/Analytics.h"
 
+#include <array>
 #include <cinttypes>
 #include <mbedtls/sha1.h>
 #include <memory>
@@ -35,12 +36,9 @@
 
 namespace
 {
-constexpr const char* ANALYTICS_ENDPOINT = "https://analytics.dolphin-emu.org/report";
+constexpr char ANALYTICS_ENDPOINT[] = "https://analytics.dolphin-emu.org/report";
 }  // namespace
 
-std::mutex DolphinAnalytics::s_instance_mutex;
-std::shared_ptr<DolphinAnalytics> DolphinAnalytics::s_instance;
-
 #if defined(ANDROID)
 static std::function<std::string(std::string)> s_get_val_func;
 void DolphinAnalytics::AndroidSetGetValFunc(std::function<std::string(std::string)> func)
@@ -57,7 +55,7 @@ DolphinAnalytics::DolphinAnalytics()
 
 std::shared_ptr<DolphinAnalytics> DolphinAnalytics::Instance()
 {
-  std::lock_guard<std::mutex> lk(s_instance_mutex);
+  std::lock_guard lk{s_instance_mutex};
   if (!s_instance)
   {
     s_instance.reset(new DolphinAnalytics());
@@ -67,7 +65,7 @@ std::shared_ptr<DolphinAnalytics> DolphinAnalytics::Instance()
 
 void DolphinAnalytics::ReloadConfig()
 {
-  std::lock_guard<std::mutex> lk(m_reporter_mutex);
+  std::lock_guard lk{m_reporter_mutex};
 
   // Install the HTTP backend if analytics support is enabled.
   std::unique_ptr<Common::AnalyticsReportingBackend> new_backend;
@@ -100,11 +98,11 @@ void DolphinAnalytics::GenerateNewIdentity()
   SConfig::GetInstance().SaveSettings();
 }
 
-std::string DolphinAnalytics::MakeUniqueId(const std::string& data)
+std::string DolphinAnalytics::MakeUniqueId(std::string_view data) const
 {
-  u8 digest[20];
+  std::array<u8, 20> digest;
-  std::string input = m_unique_id + data;
+  const auto input = std::string{m_unique_id}.append(data);
-  mbedtls_sha1(reinterpret_cast<const u8*>(input.c_str()), input.size(), digest);
+  mbedtls_sha1(reinterpret_cast<const u8*>(input.c_str()), input.size(), digest.data());
 
   // Convert to hex string and truncate to 64 bits.
   std::string out;
@@ -115,7 +113,7 @@ std::string DolphinAnalytics::MakeUniqueId(const std::string& data)
   return out;
 }
 
-void DolphinAnalytics::ReportDolphinStart(const std::string& ui_type)
+void DolphinAnalytics::ReportDolphinStart(std::string_view ui_type)
 {
   Common::AnalyticsReportBuilder builder(m_base_builder);
   builder.AddData("type", "dolphin-start");
@@ -138,12 +136,11 @@ void DolphinAnalytics::ReportGameStart()
 }
 
 // Keep in sync with enum class GameQuirk definition.
-static const char* GAME_QUIRKS_NAMES[] = {
+constexpr std::array<const char*, 2> GAME_QUIRKS_NAMES{
     "icache-matters",
     "directly-reads-wiimote-input",
 };
-static_assert(sizeof(GAME_QUIRKS_NAMES) / sizeof(GAME_QUIRKS_NAMES[0]) ==
+static_assert(GAME_QUIRKS_NAMES.size() == static_cast<u32>(GameQuirk::COUNT),
-                  static_cast<u32>(GameQuirk::COUNT),
               "Game quirks names and enum definition are out of sync.");
 
 void DolphinAnalytics::ReportGameQuirk(GameQuirk quirk)

Source/Core/Core/Analytics.h

@@ -8,6 +8,7 @@
 #include <memory>
 #include <mutex>
 #include <string>
+#include <string_view>
 #include <vector>
 
 #include "Common/Analytics.h"
@@ -49,7 +50,7 @@ public:
   void GenerateNewIdentity();
 
   // Reports a Dolphin start event.
-  void ReportDolphinStart(const std::string& ui_type);
+  void ReportDolphinStart(std::string_view ui_type);
 
   // Generates a base report for a "Game start" event. Also preseeds the
   // per-game base data.
@@ -75,7 +76,7 @@ public:
   template <typename T>
   void Send(T report)
   {
-    std::lock_guard<std::mutex> lk(m_reporter_mutex);
+    std::lock_guard lk{m_reporter_mutex};
     m_reporter.Send(report);
   }
 
@@ -88,7 +89,7 @@ private:
   // Returns a unique ID derived on the global unique ID, hashed with some
   // report-specific data. This avoid correlation between different types of
   // events.
-  std::string MakeUniqueId(const std::string& data);
+  std::string MakeUniqueId(std::string_view data) const;
 
   // Unique ID. This should never leave the application. Only used derived
   // values created by MakeUniqueId.
@@ -126,6 +127,6 @@ private:
 
   // Shared pointer in order to allow for multithreaded use of the instance and
   // avoid races at reinitialization time.
-  static std::mutex s_instance_mutex;
+  static inline std::mutex s_instance_mutex;
-  static std::shared_ptr<DolphinAnalytics> s_instance;
+  static inline std::shared_ptr<DolphinAnalytics> s_instance;
 };