Merge pull request xbmc#25449 from Rechi/cppcheck/performance

[cppcheck] fix invocation & performance checks
ace20022 · Aug 2, 2024 · 78100c5 · 78100c5
2 parents 588bdf2 + 8cdd508
commit 78100c5
Show file tree

Hide file tree

Showing 55 changed files with 111 additions and 228 deletions.
diff --git a/cmake/modules/buildtools/FindCppcheck.cmake b/cmake/modules/buildtools/FindCppcheck.cmake
@@ -21,7 +21,10 @@ if(CPPCHECK_FOUND)
   # but cppcheck doesn't support Objective-C and Objective-C++.
   # CMake >= 3.16 added support for Objective-C and Objective-C++ language,
   # but doesn't support OBJC and OBJCXX for <LANG>_CLANG_TIDY.
-  file(WRITE "${CMAKE_BINARY_DIR}/cppcheck" "case \"$@\" in *.m|*.mm) exit 0; esac\nexec \"${CPPCHECK_EXECUTABLE}\" --enable=performance --quiet --relative-paths=\"${CMAKE_SOURCE_DIR}\" \"$@\"\n")
+  if(CPPCHECK_VERSION VERSION_GREATER_EQUAL 2.14)
+    set(check-level --check-level=exhaustive)
+  endif()
+  file(WRITE "${CMAKE_BINARY_DIR}/cppcheck" "#!/bin/sh\ncase \"$@\" in *.m|*.mm) exit 0; esac\nexec \"${CPPCHECK_EXECUTABLE}\" ${check-level} --enable=performance --quiet --relative-paths=\"${CMAKE_SOURCE_DIR}\" \"$@\"\n")
   execute_process(COMMAND chmod +x "${CMAKE_BINARY_DIR}/cppcheck")
 
   # Supports Unix Makefiles and Ninja

diff --git a/cmake/scripts/common/StaticAnalysis.cmake b/cmake/scripts/common/StaticAnalysis.cmake
@@ -15,7 +15,6 @@ if(CPPCHECK_EXECUTABLE)
             --xml-version=2
             --language=c++
             --relative-paths=${CMAKE_SOURCE_DIR}
-            --rule-file=${CMAKE_SOURCE_DIR}/tools/static-analysis/cppcheck/cppcheck-rules.xml
             --suppress-xml=${CMAKE_SOURCE_DIR}/tools/static-analysis/cppcheck/cppcheck-suppressions.xml
             --output-file=${CMAKE_BINARY_DIR}/cppcheck-result.xml
     COMMENT "Static code analysis using cppcheck")

diff --git a/tools/static-analysis/cppcheck/cppcheck-rules.xml b/tools/static-analysis/cppcheck/cppcheck-rules.xml
diff --git a/xbmc/CueDocument.cpp b/xbmc/CueDocument.cpp
@@ -233,11 +233,6 @@ void CCueDocument::GetMediaFiles(std::vector<std::string>& mediaFiles)
     mediaFiles.push_back(*it);
 }
 
-std::string CCueDocument::GetMediaTitle()
-{
-  return m_strAlbum;
-}
-
 bool CCueDocument::IsLoaded() const
 {
   return !m_tracks.empty();

diff --git a/xbmc/CueDocument.h b/xbmc/CueDocument.h
@@ -39,7 +39,7 @@ class CCueDocument
   bool ParseTag(const std::string &strContent);
   void GetSongs(VECSONGS &songs);
   std::string GetMediaPath();
-  std::string GetMediaTitle();
+  const std::string& GetMediaTitle() const { return m_strAlbum; }
   void GetMediaFiles(std::vector<std::string>& mediaFiles);
   void UpdateMediaFile(const std::string& oldMediaFile, const std::string& mediaFile);
   bool IsOneFilePerTrack() const;

diff --git a/xbmc/FileItemList.cpp b/xbmc/FileItemList.cpp
@@ -1154,7 +1154,7 @@ void CFileItemList::AddSortMethod(SortBy sortBy,
   AddSortMethod(sorting, buttonLabel, labelMasks);
 }
 
-void CFileItemList::AddSortMethod(SortDescription sortDescription,
+void CFileItemList::AddSortMethod(const SortDescription& sortDescription,
                                   int buttonLabel,
                                   const LABEL_MASKS& labelMasks)
 {

diff --git a/xbmc/FileItemList.h b/xbmc/FileItemList.h
@@ -143,7 +143,7 @@ class CFileItemList : public CFileItem
                      SortAttribute sortAttributes,
                      int buttonLabel,
                      const LABEL_MASKS& labelMasks);
-  void AddSortMethod(SortDescription sortDescription,
+  void AddSortMethod(const SortDescription& sortDescription,
                      int buttonLabel,
                      const LABEL_MASKS& labelMasks);
   bool HasSortDetails() const { return m_sortDetails.size() != 0; }

diff --git a/xbmc/TextureCacheJob.cpp b/xbmc/TextureCacheJob.cpp
@@ -97,7 +97,7 @@ bool CTextureCacheJob::CacheTexture(std::unique_ptr<CTexture>* out_texture)
 {
   IMAGE_FILES::CImageFileURL imageURL{m_url};
 
-  auto image = imageURL.GetTargetFile();
+  const auto& image = imageURL.GetTargetFile();
   m_details.updateable = ShouldCheckForChanges(imageURL.GetSpecialType(), image);
 
   if (m_details.updateable)
@@ -151,7 +151,7 @@ bool CTextureCacheJob::ResizeTexture(const std::string& url,
   result_size = 0;
 
   const IMAGE_FILES::CImageFileURL imageURL{url};
-  const auto image = imageURL.GetTargetFile();
+  const auto& image = imageURL.GetTargetFile();
   if (image.empty())
     return false;
 

diff --git a/xbmc/URL.cpp b/xbmc/URL.cpp
@@ -259,7 +259,7 @@ void CURL::Parse(std::string strURL1)
   // if [] found, let's store string inside as hostname
   // and remove that parsed part from strHostNameAndPort
   size_t iBrk = strHostNameAndPort.rfind(']');
-  if (iBrk != std::string::npos && strHostNameAndPort.find('[') == 0)
+  if (iBrk != std::string::npos && strHostNameAndPort.starts_with('['))
   {
     m_strHostName = strHostNameAndPort.substr(1, iBrk-1);
     strHostNameAndPort.erase(0, iBrk+1);

diff --git a/xbmc/addons/kodi-dev-kit/include/kodi/AddonBase.h b/xbmc/addons/kodi-dev-kit/include/kodi/AddonBase.h
@@ -448,7 +448,7 @@ class ATTR_DLL_LOCAL CSettingValue
   ///@{
 
   /// @brief To get settings value as string.
-  std::string GetString() const { return str; }
+  const std::string& GetString() const { return str; }
 
   /// @brief To get settings value as integer.
   int GetInt() const { return std::atoi(str.c_str()); }

diff --git a/xbmc/addons/kodi-dev-kit/include/kodi/addon-instance/AudioDecoder.h b/xbmc/addons/kodi-dev-kit/include/kodi/addon-instance/AudioDecoder.h
@@ -79,25 +79,25 @@ class ATTR_DLL_LOCAL AudioDecoderInfoTag
   void SetTitle(const std::string& title) { m_title = title; }
 
   /// @brief Get title name
-  std::string GetTitle() const { return m_title; }
+  const std::string& GetTitle() const { return m_title; }
 
   /// @brief Set artist name
   void SetArtist(const std::string& artist) { m_artist = artist; }
 
   /// @brief Get artist name
-  std::string GetArtist() const { return m_artist; }
+  const std::string& GetArtist() const { return m_artist; }
 
   /// @brief Set album name
   void SetAlbum(const std::string& album) { m_album = album; }
 
   /// @brief Set album name
-  std::string GetAlbum() const { return m_album; }
+  const std::string& GetAlbum() const { return m_album; }
 
   /// @brief Set album artist name
   void SetAlbumArtist(const std::string& albumArtist) { m_album_artist = albumArtist; }
 
   /// @brief Get album artist name
-  std::string GetAlbumArtist() const { return m_album_artist; }
+  const std::string& GetAlbumArtist() const { return m_album_artist; }
 
   /// @brief Set the media type of the music item.
   ///
@@ -112,13 +112,13 @@ class ATTR_DLL_LOCAL AudioDecoderInfoTag
   void SetMediaType(const std::string& mediaType) { m_media_type = mediaType; }
 
   /// @brief Get the media type of the music item.
-  std::string GetMediaType() const { return m_media_type; }
+  const std::string& GetMediaType() const { return m_media_type; }
 
   /// @brief Set genre name from music as string if present.
   void SetGenre(const std::string& genre) { m_genre = genre; }
 
   /// @brief Get genre name from music as string if present.
-  std::string GetGenre() const { return m_genre; }
+  const std::string& GetGenre() const { return m_genre; }
 
   /// @brief Set the duration of music as integer from info.
   void SetDuration(int duration) { m_duration = duration; }
@@ -142,7 +142,7 @@ class ATTR_DLL_LOCAL AudioDecoderInfoTag
   void SetDiscSubtitle(const std::string& discSubtitle) { m_disc_subtitle = discSubtitle; }
 
   /// @brief Get disk subtitle name (if present) from music info.
-  std::string GetDiscSubtitle() const { return m_disc_subtitle; }
+  const std::string& GetDiscSubtitle() const { return m_disc_subtitle; }
 
   /// @brief Set disks amount quantity (if present) from music info as integer.
   void SetDiscTotal(int discTotal) { m_disc_total = discTotal; }
@@ -155,13 +155,13 @@ class ATTR_DLL_LOCAL AudioDecoderInfoTag
   void SetReleaseDate(const std::string& releaseDate) { m_release_date = releaseDate; }
 
   /// @brief Get release date as string from music info (if present).
-  std::string GetReleaseDate() const { return m_release_date; }
+  const std::string& GetReleaseDate() const { return m_release_date; }
 
   /// @brief Set string from lyrics.
   void SetLyrics(const std::string& lyrics) { m_lyrics = lyrics; }
 
   /// @brief Get string from lyrics.
-  std::string GetLyrics() const { return m_lyrics; }
+  const std::string& GetLyrics() const { return m_lyrics; }
 
   /// @brief Set related stream samplerate.
   void SetSamplerate(int samplerate) { m_samplerate = samplerate; }
@@ -185,7 +185,7 @@ class ATTR_DLL_LOCAL AudioDecoderInfoTag
   void SetComment(const std::string& comment) { m_comment = comment; }
 
   /// @brief Get additional information comment (if present).
-  std::string GetComment() const { return m_comment; }
+  const std::string& GetComment() const { return m_comment; }
 
   /// @brief Set cover art image by path.
   ///
@@ -201,7 +201,7 @@ class ATTR_DLL_LOCAL AudioDecoderInfoTag
   /// @note Only be available if set before by @ref SetCoverArtByPath.
   /// Cannot be combined with @ref SetCoverArtByMem and @ref GetCoverArtByMem.
   ///
-  std::string GetCoverArtByPath() const { return m_cover_art_path; }
+  const std::string& GetCoverArtByPath() const { return m_cover_art_path; }
 
   /// @brief Set cover art image by memory.
   ///

diff --git a/xbmc/addons/kodi-dev-kit/include/kodi/addon-instance/ImageDecoder.h b/xbmc/addons/kodi-dev-kit/include/kodi/addon-instance/ImageDecoder.h
@@ -289,31 +289,31 @@ class ATTR_DLL_LOCAL ImageDecoderInfoTag
   }
 
   /// @brief Get camera manufacturer
-  std::string GetCameraManufacturer() const { return m_camera_manufacturer; }
+  const std::string& GetCameraManufacturer() const { return m_camera_manufacturer; }
 
   /// @brief Set camera model
   void SetCameraModel(const std::string& camera_model) { m_camera_model = camera_model; }
 
   /// @brief Get camera model
-  std::string GetCameraModel() const { return m_camera_model; }
+  const std::string& GetCameraModel() const { return m_camera_model; }
 
   /// @brief Set author
   void SetAuthor(const std::string& author) { m_author = author; }
 
   /// @brief Get author
-  std::string GetAuthor() const { return m_author; }
+  const std::string& GetAuthor() const { return m_author; }
 
   /// @brief Set description
   void SetDescription(const std::string& description) { m_description = description; }
 
   /// @brief Get description
-  std::string GetDescription() const { return m_description; }
+  const std::string& GetDescription() const { return m_description; }
 
   /// @brief Set copyright
   void SetCopyright(const std::string& copyright) { m_copyright = copyright; }
 
   /// @brief Get copyright
-  std::string GetCopyright() const { return m_copyright; }
+  const std::string& GetCopyright() const { return m_copyright; }
 
   ///@}
 

diff --git a/xbmc/application/ApplicationPowerHandling.h b/xbmc/application/ApplicationPowerHandling.h
@@ -42,7 +42,7 @@ class CApplicationPowerHandling : public IApplicationComponent
   void SetScreenSaverLockFailed() { m_iScreenSaveLock = -1; }
   void SetScreenSaverUnlocked() { m_iScreenSaveLock = 1; }
   void StopScreenSaverTimer();
-  std::string ScreensaverIdInUse() const { return m_screensaverIdInUse; }
+  const std::string& ScreensaverIdInUse() const { return m_screensaverIdInUse; }
 
   bool GetRenderGUI() const { return m_renderGUI; }
   void SetRenderGUI(bool renderGUI);

diff --git a/xbmc/cores/AudioEngine/Sinks/osx/AEDeviceEnumerationOSX.h b/xbmc/cores/AudioEngine/Sinks/osx/AEDeviceEnumerationOSX.h
@@ -90,7 +90,7 @@ class AEDeviceEnumerationOSX
   * @brief Returns the device name which belongs to m_deviceID without any stream/source suffixes
   * @return the CA device name
   */
-  std::string   GetMasterDeviceName() const { return m_deviceName; }
+  const std::string& GetMasterDeviceName() const { return m_deviceName; }
 
   /*!
   * @brief Tries to return a proper channelmap from CA in a format AE understands

diff --git a/xbmc/cores/AudioEngine/Sinks/pipewire/PipewireGlobal.h b/xbmc/cores/AudioEngine/Sinks/pipewire/PipewireGlobal.h
@@ -35,15 +35,15 @@ class CPipewireGlobal
     return *this;
   }
 
-  std::string GetName() const { return m_name; }
+  const std::string& GetName() const { return m_name; }
 
   CPipewireGlobal& SetDescription(const std::string& description)
   {
     m_description = description;
     return *this;
   }
 
-  std::string GetDescription() const { return m_description; }
+  const std::string& GetDescription() const { return m_description; }
 
   CPipewireGlobal& SetID(uint32_t id)
   {
@@ -67,7 +67,7 @@ class CPipewireGlobal
     return *this;
   }
 
-  std::string GetType() const { return m_type; }
+  const std::string& GetType() const { return m_type; }
 
   CPipewireGlobal& SetVersion(uint32_t version)
   {

diff --git a/xbmc/cores/RetroPlayer/rendering/RenderVideoSettings.h b/xbmc/cores/RetroPlayer/rendering/RenderVideoSettings.h
@@ -46,7 +46,7 @@ class CRenderVideoSettings
   unsigned int GetRenderRotation() const { return m_rotationDegCCW; }
   void SetRenderRotation(unsigned int rotationDegCCW) { m_rotationDegCCW = rotationDegCCW; }
 
-  std::string GetPixels() const { return m_pixelPath; }
+  const std::string& GetPixels() const { return m_pixelPath; }
   void SetPixels(const std::string& pixelPath) { m_pixelPath = pixelPath; }
   void ResetPixels();
 

diff --git a/xbmc/favourites/FavouritesURL.h b/xbmc/favourites/FavouritesURL.h
@@ -36,7 +36,7 @@ class CFavouritesURL
 
   virtual ~CFavouritesURL() = default;
 
-  std::string GetURL() const { return m_path; }
+  const std::string& GetURL() const { return m_path; }
 
   bool IsValid() const { return m_valid && m_exec.IsValid(); }
 
@@ -45,10 +45,10 @@ class CFavouritesURL
   std::string GetExecString() const { return m_exec.GetExecString(); }
   Action GetAction() const { return m_action; }
   std::vector<std::string> GetParams() const { return m_exec.GetParams(); }
-  std::string GetTarget() const { return m_target; }
+  const std::string& GetTarget() const { return m_target; }
   int GetWindowID() const { return m_windowId; }
-  std::string GetActionLabel() const { return m_actionLabel; }
-  std::string GetProviderLabel() const { return m_providerLabel; }
+  const std::string& GetActionLabel() const { return m_actionLabel; }
+  const std::string& GetProviderLabel() const { return m_providerLabel; }
 
 private:
   bool Parse(CFavouritesURL::Action action, const std::vector<std::string>& params);

diff --git a/xbmc/filesystem/CurlFile.cpp b/xbmc/filesystem/CurlFile.cpp
@@ -1929,11 +1929,6 @@ void CCurlFile::SetRequestHeader(const std::string& header, long value)
   m_requestheaders[header] = std::to_string(value);
 }
 
-std::string CCurlFile::GetURL(void)
-{
-  return m_url;
-}
-
 std::string CCurlFile::GetRedirectURL()
 {
   return GetInfoString(CURLINFO_REDIRECT_URL);

diff --git a/xbmc/filesystem/CurlFile.h b/xbmc/filesystem/CurlFile.h
@@ -80,7 +80,7 @@ namespace XFILE
       void SetBufferSize(unsigned int size);
 
       const CHttpHeader& GetHttpHeader() const { return m_state->m_httpheader; }
-      std::string GetURL(void);
+      const std::string& GetURL() const { return m_url; }
       std::string GetRedirectURL();
 
       /* static function that will get content type of a file */

diff --git a/xbmc/filesystem/FTPParse.cpp b/xbmc/filesystem/FTPParse.cpp
@@ -17,11 +17,6 @@ CFTPParse::CFTPParse()
   m_size = 0;
 }
 
-std::string CFTPParse::getName()
-{
-  return m_name;
-}
-
 int CFTPParse::getFlagtrycwd()
 {
   return m_flagtrycwd;

diff --git a/xbmc/filesystem/FTPParse.h b/xbmc/filesystem/FTPParse.h
@@ -17,7 +17,7 @@ class CFTPParse
 public:
   CFTPParse();
   int FTPParse(const std::string& str);
-  std::string getName();
+  const std::string& getName() const { return m_name; }
   int getFlagtrycwd();
   int getFlagtryretr();
   uint64_t getSize();

diff --git a/xbmc/imagefiles/ImageCacheCleaner.cpp b/xbmc/imagefiles/ImageCacheCleaner.cpp
@@ -22,7 +22,7 @@ std::optional<IMAGE_FILES::CImageCacheCleaner> CImageCacheCleaner::Create()
 {
   auto result = CImageCacheCleaner();
   if (result.m_valid)
-    return std::move(result);
+    return result;
   return {};
 }
 

diff --git a/xbmc/imagefiles/ImageFileURL.h b/xbmc/imagefiles/ImageFileURL.h
@@ -58,10 +58,10 @@ class CImageFileURL
   */
   std::string ToCacheKey() const;
 
-  std::string GetTargetFile() const { return m_filePath; }
+  const std::string& GetTargetFile() const { return m_filePath; }
 
   bool IsSpecialImage() const { return !m_specialType.empty(); }
-  std::string GetSpecialType() const { return m_specialType; }
+  const std::string& GetSpecialType() const { return m_specialType; }
 
   bool flipped{false};
 

diff --git a/xbmc/input/InputManager.cpp b/xbmc/input/InputManager.cpp
@@ -704,7 +704,7 @@ bool CInputManager::AlwaysProcess(const CAction& action)
     const CExecString exec(action.GetName());
     if (exec.IsValid())
     {
-      const std::string builtInFunction = exec.GetFunction();
+      const std::string& builtInFunction = exec.GetFunction();
 
       // should this button be handled normally or just cancel the screensaver?
       if (builtInFunction == "powerdown" || builtInFunction == "reboot" ||