Merge pull request #23390 from neo1973/backport_23283

[BP] SysfsPath: Have CSysfsPath::Get() return a std::optional because reads can fail

3 files changed, 53 insertions, 29 deletions

diff --git a/xbmc/platform/linux/CPUInfoLinux.cpp b/xbmc/platform/linux/CPUInfoLinux.cpp
index 88aad50028..ef34bbd0c6 100644
--- a/xbmc/platform/linux/CPUInfoLinux.cpp
+++ b/xbmc/platform/linux/CPUInfoLinux.cpp
@@ -13,6 +13,7 @@
 
 #include "platform/linux/SysfsPath.h"
 
+#include <exception>
 #include <fstream>
 #include <regex>
 #include <sstream>
@@ -71,23 +72,23 @@ CCPUInfoLinux::CCPUInfoLinux()
 {
   CSysfsPath machinePath{"/sys/bus/soc/devices/soc0/machine"};
   if (machinePath.Exists())
-    m_cpuHardware = machinePath.Get<std::string>();
+    m_cpuHardware = machinePath.Get<std::string>().value_or("");
 
   CSysfsPath familyPath{"/sys/bus/soc/devices/soc0/family"};
   if (familyPath.Exists())
-    m_cpuSoC = familyPath.Get<std::string>();
+    m_cpuSoC = familyPath.Get<std::string>().value_or("");
 
   CSysfsPath socPath{"/sys/bus/soc/devices/soc0/soc_id"};
   if (socPath.Exists())
-    m_cpuSoC += " " + socPath.Get<std::string>();
+    m_cpuSoC += " " + socPath.Get<std::string>().value_or("");
 
   CSysfsPath revisionPath{"/sys/bus/soc/devices/soc0/revision"};
   if (revisionPath.Exists())
-    m_cpuRevision += revisionPath.Get<std::string>();
+    m_cpuRevision = revisionPath.Get<std::string>().value_or("");
 
   CSysfsPath serialPath{"/sys/bus/soc/devices/soc0/serial_number"};
   if (serialPath.Exists())
-    m_cpuSerial += serialPath.Get<std::string>();
+    m_cpuSerial = serialPath.Get<std::string>().value_or("");
 
   const std::string freqStr{"/sys/devices/system/cpu/cpu0/cpufreq/scaling_cur_freq"};
   CSysfsPath freqPath{freqStr};
@@ -109,7 +110,7 @@ CCPUInfoLinux::CCPUInfoLinux()
 
     auto name = path.Get<std::string>();
 
-    if (name.empty())
+    if (!name.has_value())
       continue;
 
     for (const auto& module : modules)
@@ -353,8 +354,8 @@ float CCPUInfoLinux::GetCPUFrequency()
   if (m_freqPath.empty())
     return 0;
 
-  CSysfsPath path{m_freqPath};
-  return path.Get<float>() / 1000.0f;
+  auto freq = CSysfsPath(m_freqPath).Get<float>();
+  return freq.has_value() ? *freq / 1000.0f : 0.0f;
 }
 
 bool CCPUInfoLinux::GetTemperature(CTemperature& temperature)
@@ -365,8 +366,11 @@ bool CCPUInfoLinux::GetTemperature(CTemperature& temperature)
   if (m_tempPath.empty())
     return false;
 
-  CSysfsPath path{m_tempPath};
-  double value = path.Get<double>() / 1000.0;
+  auto temp = CSysfsPath(m_tempPath).Get<double>();
+  if (!temp.has_value())
+    return false;
+
+  double value = *temp / 1000.0;
 
   temperature = CTemperature::CreateFromCelsius(value);
   temperature.SetValid(true);
diff --git a/xbmc/platform/linux/SysfsPath.cpp b/xbmc/platform/linux/SysfsPath.cpp
index 6b6d78a8a8..af4f9cb401 100644
--- a/xbmc/platform/linux/SysfsPath.cpp
+++ b/xbmc/platform/linux/SysfsPath.cpp
@@ -8,6 +8,8 @@
 
 #include "SysfsPath.h"
 
+#include <exception>
+
 bool CSysfsPath::Exists()
 {
   std::ifstream file(m_path);
@@ -19,19 +21,27 @@ bool CSysfsPath::Exists()
 }
 
 template<>
-std::string CSysfsPath::Get()
+std::optional<std::string> CSysfsPath::Get()
 {
-  std::ifstream file(m_path);
+  try
+  {
+    std::ifstream file(m_path);
 
-  std::string value;
+    std::string value;
 
-  std::getline(file, value);
+    std::getline(file, value);
 
-  if (file.bad())
+    if (file.bad())
+    {
+      CLog::LogF(LOGERROR, "error reading from '{}'", m_path);
+      return std::nullopt;
+    }
+
+    return value;
+  }
+  catch (const std::exception& e)
   {
-    CLog::LogF(LOGERROR, "error reading from '{}'", m_path);
-    throw std::runtime_error("error reading from " + m_path);
+    CLog::LogF(LOGERROR, "exception reading from '{}': {}", m_path, e.what());
+    return std::nullopt;
   }
-
-  return value;
 }
diff --git a/xbmc/platform/linux/SysfsPath.h b/xbmc/platform/linux/SysfsPath.h
index 72c7fe017e..3c19703be2 100644
--- a/xbmc/platform/linux/SysfsPath.h
+++ b/xbmc/platform/linux/SysfsPath.h
@@ -10,7 +10,9 @@
 
 #include "utils/log.h"
 
+#include <exception>
 #include <fstream>
+#include <optional>
 #include <string>
 
 class CSysfsPath
@@ -23,21 +25,29 @@ public:
   bool Exists();
 
   template<typename T>
-  T Get()
+  std::optional<T> Get()
   {
-    std::ifstream file(m_path);
+    try
+    {
+      std::ifstream file(m_path);
+
+      T value;
 
-    T value;
+      file >> value;
 
-    file >> value;
+      if (file.bad())
+      {
+        CLog::LogF(LOGERROR, "error reading from '{}'", m_path);
+        return std::nullopt;
+      }
 
-    if (file.bad())
+      return value;
+    }
+    catch (const std::exception& e)
     {
-      CLog::LogF(LOGERROR, "error reading from '{}'", m_path);
-      throw std::runtime_error("error reading from " + m_path);
+      CLog::LogF(LOGERROR, "exception reading from '{}': {}", m_path, e.what());
+      return std::nullopt;
     }
-
-    return value;
   }
 
 private:
@@ -45,4 +55,4 @@ private:
 };
 
 template<>
-std::string CSysfsPath::Get<std::string>();
+std::optional<std::string> CSysfsPath::Get<std::string>();