SysfsUtils: cleanup and improve interface according to the documentation

as written in the kernel docs:

sysfs allocates a buffer of size (PAGE_SIZE) and passes it to the
method. Sysfs will call the method exactly once for each read or
write.

for more info please consult the kernel documentation:

https://www.kernel.org/doc/Documentation/filesystems/sysfs.txt

6 files changed, 73 insertions, 171 deletions

diff --git a/xbmc/platform/linux/CPUInfoLinux.cpp b/xbmc/platform/linux/CPUInfoLinux.cpp
index e5e6ac249f..22aa043c0e 100644
--- a/xbmc/platform/linux/CPUInfoLinux.cpp
+++ b/xbmc/platform/linux/CPUInfoLinux.cpp
@@ -68,23 +68,17 @@ std::shared_ptr<CCPUInfo> CCPUInfo::GetCPUInfo()
 
 CCPUInfoLinux::CCPUInfoLinux()
 {
-  // new socs use the sysfs soc interface to describe the hardware
-  if (SysfsUtils::Has("/sys/bus/soc/devices/soc0"))
-  {
-    std::string machine;
-    std::string family;
-    std::string socId;
-    if (SysfsUtils::Has("/sys/bus/soc/devices/soc0/machine"))
-      SysfsUtils::GetString("/sys/bus/soc/devices/soc0/machine", machine);
-    if (SysfsUtils::Has("/sys/bus/soc/devices/soc0/family"))
-      SysfsUtils::GetString("/sys/bus/soc/devices/soc0/family", family);
-    if (SysfsUtils::Has("/sys/bus/soc/devices/soc0/soc_id"))
-      SysfsUtils::GetString("/sys/bus/soc/devices/soc0/soc_id", socId);
-    if (m_cpuHardware.empty() && !machine.empty())
-      m_cpuHardware = machine;
-    if (!family.empty() && !socId.empty())
-      m_cpuSoC = family + " " + socId;
-  }
+  CSysfsPath machinePath{"/sys/bus/soc/devices/soc0/machine"};
+  if (machinePath.Exists())
+    m_cpuHardware = machinePath.Get<std::string>();
+
+  CSysfsPath familyPath{"/sys/bus/soc/devices/soc0/family"};
+  if (familyPath.Exists())
+    m_cpuSoC = familyPath.Get<std::string>();
+
+  CSysfsPath socPath{"/sys/bus/soc/devices/soc0/soc_id"};
+  if (socPath.Exists())
+    m_cpuSoC += " " + socPath.Get<std::string>();
 
   m_cpuCount = sysconf(_SC_NPROCESSORS_ONLN);
 
@@ -284,34 +278,26 @@ int CCPUInfoLinux::GetUsedPercentage()
 
 float CCPUInfoLinux::GetCPUFrequency()
 {
-  int value{-1};
-  if (SysfsUtils::Has("/sys/devices/system/cpu/cpu0/cpufreq/scaling_cur_freq"))
-    SysfsUtils::GetInt("/sys/devices/system/cpu/cpu0/cpufreq/scaling_cur_freq", value);
+  float value{0};
+  CSysfsPath path("/sys/devices/system/cpu/cpu0/cpufreq/scaling_cur_freq");
 
-  value /= 1000.0;
+  if (path.Exists())
+    value = path.Get<float>() / 1000.0;
 
   return value;
 }
 
 bool CCPUInfoLinux::GetTemperature(CTemperature& temperature)
 {
-  if (!SysfsUtils::Has("/sys/class/hwmon/hwmon0/temp1_input"))
-    return CCPUInfoPosix::GetTemperature(temperature);
-
-  int value{-1};
-  char scale{'c'};
+  int value{0};
+  CSysfsPath path("/sys/class/hwmon/hwmon0/temp1_input");
 
-  SysfsUtils::GetInt("/sys/class/hwmon/hwmon0/temp1_input", value);
-  value = value / 1000.0;
-  scale = 'c';
+  if (!path.Exists())
+    return CCPUInfoPosix::GetTemperature(temperature);
 
-  if (scale == 'C' || scale == 'c')
-    temperature = CTemperature::CreateFromCelsius(value);
-  else if (scale == 'F' || scale == 'f')
-    temperature = CTemperature::CreateFromFahrenheit(value);
-  else
-    return false;
+  value = path.Get<int>() / 1000.0;
 
+  temperature = CTemperature::CreateFromCelsius(value);
   temperature.SetValid(true);
 
   return true;
diff --git a/xbmc/utils/CMakeLists.txt b/xbmc/utils/CMakeLists.txt
index 826915edb8..d13f1ae3e4 100644
--- a/xbmc/utils/CMakeLists.txt
+++ b/xbmc/utils/CMakeLists.txt
@@ -59,7 +59,6 @@ set(SOURCES ActorProtocol.cpp
             StreamUtils.cpp
             StringUtils.cpp
             StringValidation.cpp
-            SysfsUtils.cpp
             SystemInfo.cpp
             Temperature.cpp
             TextSearch.cpp
@@ -151,7 +150,6 @@ set(HEADERS ActorProtocol.h
             StreamUtils.h
             StringUtils.h
             StringValidation.h
-            SysfsUtils.h
             SystemInfo.h
             Temperature.h
             TextSearch.h
@@ -187,6 +185,11 @@ if(NOT CORE_SYSTEM_NAME STREQUAL windows AND NOT CORE_SYSTEM_NAME STREQUAL windo
   list(APPEND HEADERS GLUtils.h)
 endif()
 
+if(CORE_SYSTEM_NAME STREQUAL linux)
+  list(APPEND SOURCES SysfsUtils.cpp)
+  list(APPEND HEADERS SysfsUtils.h)
+endif()
+
 if(CORE_PLATFORM_NAME_LC STREQUAL gbm)
   list(APPEND SOURCES EGLImage.cpp)
   list(APPEND HEADERS EGLImage.h)
diff --git a/xbmc/utils/SysfsUtils.cpp b/xbmc/utils/SysfsUtils.cpp
index 044d4c4b6c..b27ea50f53 100644
--- a/xbmc/utils/SysfsUtils.cpp
+++ b/xbmc/utils/SysfsUtils.cpp
@@ -7,114 +7,13 @@
  */
 
 #include "SysfsUtils.h"
-#include "utils/log.h"
-#include "utils/StringUtils.h"
 
-#include <unistd.h>
-#include <stdio.h>
-#include <stdlib.h>
-#include <fcntl.h>
-#include <string.h>
-
-#ifdef TARGET_WINDOWS_STORE
-#include <io.h>
-#endif
-
-int SysfsUtils::SetString(const std::string& path, const std::string& valstr)
-{
-  int fd = open(path.c_str(), O_RDWR, 0644);
-  int ret = 0;
-  if (fd >= 0)
-  {
-    if (write(fd, valstr.c_str(), valstr.size()) < 0)
-      ret = -1;
-    close(fd);
-  }
-  if (ret)
-    CLog::Log(LOGERROR, "%s: error writing %s",__FUNCTION__, path.c_str());
-
-  return ret;
-}
-
-int SysfsUtils::GetString(const std::string& path, std::string& valstr)
+bool CSysfsPath::Exists()
 {
-  int len;
-  char buf[256] = {0};
+  std::ifstream file(m_path);
 
-  int fd = open(path.c_str(), O_RDONLY);
-  if (fd >= 0)
-  {
-    valstr.clear();
-    while ((len = read(fd, buf, 256)) > 0)
-      valstr.append(buf, len);
-    close(fd);
-
-    StringUtils::Trim(valstr);
-
-    return 0;
-  }
-
-  CLog::Log(LOGERROR, "%s: error reading %s",__FUNCTION__, path.c_str());
-  valstr = "fail";
-  return -1;
-}
-
-int SysfsUtils::SetInt(const std::string& path, const int val)
-{
-  int fd = open(path.c_str(), O_RDWR, 0644);
-  int ret = 0;
-  if (fd >= 0)
-  {
-    char bcmd[16];
-    sprintf(bcmd, "%d", val);
-    if (write(fd, bcmd, strlen(bcmd)) < 0)
-      ret = -1;
-    close(fd);
-  }
-  if (ret)
-    CLog::Log(LOGERROR, "%s: error writing %s",__FUNCTION__, path.c_str());
+  if (!file.is_open())
+    return false;
 
-  return ret;
-}
-
-int SysfsUtils::GetInt(const std::string& path, int& val)
-{
-  int fd = open(path.c_str(), O_RDONLY);
-  int ret = 0;
-  if (fd >= 0)
-  {
-    char bcmd[16];
-    if (read(fd, bcmd, sizeof(bcmd)) < 0)
-      ret = -1;
-    else
-      val = strtol(bcmd, NULL, 16);
-
-    close(fd);
-  }
-  if (ret)
-    CLog::Log(LOGERROR, "%s: error reading %s",__FUNCTION__, path.c_str());
-
-  return ret;
-}
-
-bool SysfsUtils::Has(const std::string &path)
-{
-  int fd = open(path.c_str(), O_RDONLY);
-  if (fd >= 0)
-  {
-    close(fd);
-    return true;
-  }
-  return false;
-}
-
-bool SysfsUtils::HasRW(const std::string &path)
-{
-  int fd = open(path.c_str(), O_RDWR);
-  if (fd >= 0)
-  {
-    close(fd);
-    return true;
-  }
-  return false;
+  return true;
 }
diff --git a/xbmc/utils/SysfsUtils.h b/xbmc/utils/SysfsUtils.h
index b173a456bc..499d3d4a93 100644
--- a/xbmc/utils/SysfsUtils.h
+++ b/xbmc/utils/SysfsUtils.h
@@ -8,15 +8,38 @@
 
 #pragma once
 
+#include "utils/log.h"
+
+#include <fstream>
 #include <string>
 
-class SysfsUtils
+class CSysfsPath
 {
 public:
-  static int SetString(const std::string& path, const std::string& valstr);
-  static int GetString(const std::string& path, std::string& valstr);
-  static int SetInt(const std::string& path, const int val);
-  static int GetInt(const std::string& path, int& val);
-  static bool Has(const std::string& path);
-  static bool HasRW(const std::string &path);
+  CSysfsPath() = default;
+  CSysfsPath(const std::string& path) : m_path(path) {}
+  ~CSysfsPath() = default;
+
+  bool Exists();
+
+  template<typename T>
+  T Get()
+  {
+    std::ifstream file(m_path);
+
+    T value;
+
+    file >> value;
+
+    if (file.bad())
+    {
+      CLog::LogF(LOGERROR, "error reading from '{}'", m_path);
+      throw std::runtime_error("error reading from " + m_path);
+    }
+
+    return value;
+  }
+
+private:
+  std::string m_path;
 };
diff --git a/xbmc/utils/SystemInfo.cpp b/xbmc/utils/SystemInfo.cpp
index a85c10d185..812d0c8e06 100644
--- a/xbmc/utils/SystemInfo.cpp
+++ b/xbmc/utils/SystemInfo.cpp
@@ -723,19 +723,10 @@ std::string CSysInfo::GetManufacturerName(void)
     auto manufacturer = eas.SystemManufacturer();
     g_charsetConverter.wToUTF8(std::wstring(manufacturer.c_str()), manufName);
 #elif defined(TARGET_LINUX)
-    if (SysfsUtils::Has("/sys/bus/soc/devices/soc0/family"))
-    {
-      std::string family;
-      SysfsUtils::GetString("/sys/bus/soc/devices/soc0/family", family);
-      if (SysfsUtils::Has("/sys/bus/soc/devices/soc0/soc_id"))
-      {
-        std::string soc_id;
-        SysfsUtils::GetString("/sys/bus/soc/devices/soc0/soc_id", soc_id);
-        manufName = family + " " + soc_id;
-      }
-      else
-        manufName = family;
-    }
+
+    auto cpuInfo = CServiceBroker::GetCPUInfo();
+    manufName = cpuInfo->GetCPUSoC();
+
 #elif defined(TARGET_WINDOWS)
     // We just don't care, might be useful on embedded
 #endif
@@ -770,8 +761,8 @@ std::string CSysInfo::GetModelName(void)
     auto manufacturer = eas.SystemProductName();
     g_charsetConverter.wToUTF8(std::wstring(manufacturer.c_str()), modelName);
 #elif defined(TARGET_LINUX)
-    if (SysfsUtils::Has("/sys/bus/soc/devices/soc0/machine"))
-      SysfsUtils::GetString("/sys/bus/soc/devices/soc0/machine", modelName);
+    auto cpuInfo = CServiceBroker::GetCPUInfo();
+    modelName = cpuInfo->GetCPUHardware();
 #elif defined(TARGET_WINDOWS)
     // We just don't care, might be useful on embedded
 #endif
diff --git a/xbmc/utils/test/TestSystemInfo.cpp b/xbmc/utils/test/TestSystemInfo.cpp
index b80d32bc33..1f2b0a10f3 100644
--- a/xbmc/utils/test/TestSystemInfo.cpp
+++ b/xbmc/utils/test/TestSystemInfo.cpp
@@ -6,9 +6,11 @@
  *  See LICENSES/README.md for more information.
  */
 
-#include "utils/SystemInfo.h"
-#include "settings/Settings.h"
 #include "GUIInfoManager.h"
+#include "ServiceBroker.h"
+#include "settings/Settings.h"
+#include "utils/CPUInfo.h"
+#include "utils/SystemInfo.h"
 #if defined(TARGET_WINDOWS)
 #include "platform/win32/CharsetConverter.h"
 #endif
@@ -18,10 +20,8 @@
 class TestSystemInfo : public testing::Test
 {
 protected:
-  TestSystemInfo()
-  = default;
-  ~TestSystemInfo() override
-  = default;
+  TestSystemInfo() { CServiceBroker::RegisterCPUInfo(CCPUInfo::GetCPUInfo()); }
+  ~TestSystemInfo() { CServiceBroker::UnregisterCPUInfo(); }
 };
 
 TEST_F(TestSystemInfo, Print_System_Info)