Merge pull request #9271 from leoetlino/warnings

Fix several warnings and only enable extra warnings for our own code

CMakeLists.txt

@@ -255,23 +255,6 @@ if(CMAKE_C_COMPILER_ID MATCHES "MSVC")
   string(APPEND CMAKE_EXE_LINKER_FLAGS " /NXCOMPAT")
 else()
   add_definitions(-D_DEFAULT_SOURCE)
-  check_and_add_flag(HAVE_WALL -Wall)
-  # TODO: would like these but they produce overwhelming amounts of warnings
-  #check_and_add_flag(EXTRA -Wextra)
-  #check_and_add_flag(MISSING_FIELD_INITIALIZERS -Wmissing-field-initializers)
-  #check_and_add_flag(SWITCH_DEFAULT -Wswitch-default)
-  #check_and_add_flag(FLOAT_EQUAL -Wfloat-equal)
-  #check_and_add_flag(CONVERSION -Wconversion)
-  #check_and_add_flag(ZERO_AS_NULL_POINTER_CONSTANT -Wzero-as-null-pointer-constant)
-  check_and_add_flag(TYPE_LIMITS -Wtype-limits)
-  check_and_add_flag(SIGN_COMPARE -Wsign-compare)
-  check_and_add_flag(IGNORED_QUALIFIERS -Wignored-qualifiers)
-  check_and_add_flag(UNINITIALIZED -Wuninitialized)
-  check_and_add_flag(LOGICAL_OP -Wlogical-op)
-  check_and_add_flag(SHADOW -Wshadow)
-  check_and_add_flag(INIT_SELF -Winit-self)
-  check_and_add_flag(MISSING_DECLARATIONS -Wmissing-declarations)
-  check_and_add_flag(MISSING_VARIABLE_DECLARATIONS -Wmissing-variable-declarations)
 
   # gcc uses some optimizations which might break stuff without this flag
   check_and_add_flag(NO_STRICT_ALIASING -fno-strict-aliasing)

Source/CMakeLists.txt

@@ -62,10 +62,37 @@ if (MSVC)
   # Use PCH
   add_subdirectory(PCH)
   add_definitions(/I${PCH_DIRECTORY})
-  add_definitions(/Yu${PCH_PATH}) 
+  add_definitions(/Yu${PCH_PATH})
 
   # Don't include timestamps in binaries
   add_link_options(/Brepro)
+else()
+  check_and_add_flag(HAVE_WALL -Wall)
+  # TODO: would like these but they produce overwhelming amounts of warnings
+  #check_and_add_flag(EXTRA -Wextra)
+  #check_and_add_flag(MISSING_FIELD_INITIALIZERS -Wmissing-field-initializers)
+  #check_and_add_flag(SWITCH_DEFAULT -Wswitch-default)
+  #check_and_add_flag(FLOAT_EQUAL -Wfloat-equal)
+  #check_and_add_flag(CONVERSION -Wconversion)
+  #check_and_add_flag(ZERO_AS_NULL_POINTER_CONSTANT -Wzero-as-null-pointer-constant)
+  check_and_add_flag(TYPE_LIMITS -Wtype-limits)
+  check_and_add_flag(SIGN_COMPARE -Wsign-compare)
+  check_and_add_flag(IGNORED_QUALIFIERS -Wignored-qualifiers)
+  check_and_add_flag(UNINITIALIZED -Wuninitialized)
+  check_and_add_flag(LOGICAL_OP -Wlogical-op)
+  check_and_add_flag(SHADOW -Wshadow)
+  check_and_add_flag(INIT_SELF -Winit-self)
+  check_and_add_flag(MISSING_DECLARATIONS -Wmissing-declarations)
+  check_and_add_flag(MISSING_VARIABLE_DECLARATIONS -Wmissing-variable-declarations)
+
+  # Disable -Wstringop-truncation warnings as they result in many false positives.
+  # In most (all?) cases where std::strncpy is used, we want to fill the entire buffer
+  # or match emulated code that also ignores the null terminator, so the warnings are not useful.
+  # Given that Dolphin itself mostly uses std::string, they do not really help catch any bugs.
+  check_cxx_compiler_flag(-Wstringop-truncation HAS_STRINGOP_TRUNCATION_WARNING)
+  if (HAS_STRINGOP_TRUNCATION_WARNING)
+    check_and_add_flag(NO_STRINGOP_TRUNCATION -Wno-stringop-truncation)
+  endif()
 endif()
 
 # These aren't actually needed for C11/C++11

Source/Core/Common/Hash.cpp

@@ -99,7 +99,7 @@ u32 HashEctor(const u8* ptr, size_t length)
 {
   u32 crc = 0;
 
-  for (int i = 0; i < length; i++)
+  for (size_t i = 0; i < length; i++)
   {
     crc ^= ptr[i];
     crc = (crc << 3) | (crc >> 29);

Source/Core/Core/GeckoCode.h

@@ -16,7 +16,7 @@ namespace Gecko
 class GeckoCode
 {
 public:
-  GeckoCode() : enabled(false) {}
+  GeckoCode() = default;
   struct Code
   {
     u32 address = 0;
@@ -28,8 +28,8 @@ public:
   std::string name, creator;
   std::vector<std::string> notes;
 
-  bool enabled;
+  bool enabled = false;
-  bool user_defined;
+  bool user_defined = false;
 
   bool Exist(u32 address, u32 data) const;
 };

Source/Core/Core/IOS/Network/Socket.cpp

@@ -199,6 +199,8 @@ s32 WiiSocket::Shutdown(u32 how)
       if (shut_write)
         op.Abort(-SO_ENOTCONN);
       break;
+    default:
+      break;
     }
   }
   return ret;

Source/Core/DolphinQt/NetPlay/NetPlayDialog.cpp

@@ -1049,10 +1049,10 @@ NetPlayDialog::FindGameFile(const NetPlay::SyncIdentifier& sync_identifier,
       RunOnObject(this, [this, &sync_identifier, found] {
         for (int i = 0; i < m_game_list_model.rowCount(QModelIndex()); i++)
         {
-          auto game_file = m_game_list_model.GetGameFile(i);
+          auto file = m_game_list_model.GetGameFile(i);
-          *found = std::min(*found, game_file->CompareSyncIdentifier(sync_identifier));
+          *found = std::min(*found, file->CompareSyncIdentifier(sync_identifier));
           if (*found == NetPlay::SyncIdentifierComparison::SameGame)
-            return game_file;
+            return file;
         }
         return static_cast<std::shared_ptr<const UICommon::GameFile>>(nullptr);
       });

Source/Core/DolphinQt/QtUtils/UTF8CodePointCountValidator.cpp

@@ -6,7 +6,7 @@
 
 #include "Common/StringUtil.h"
 
-UTF8CodePointCountValidator::UTF8CodePointCountValidator(int max_count, QObject* parent)
+UTF8CodePointCountValidator::UTF8CodePointCountValidator(std::size_t max_count, QObject* parent)
     : QValidator(parent), m_max_count(max_count)
 {
 }

Source/Core/DolphinQt/QtUtils/UTF8CodePointCountValidator.h

@@ -4,6 +4,8 @@
 
 #pragma once
 
+#include <cstddef>
+
 #include <QString>
 #include <QValidator>
 
@@ -11,9 +13,10 @@ class UTF8CodePointCountValidator : public QValidator
 {
   Q_OBJECT
 public:
-  explicit UTF8CodePointCountValidator(int max_count, QObject* parent = nullptr);
+  explicit UTF8CodePointCountValidator(std::size_t max_count, QObject* parent = nullptr);
 
   QValidator::State validate(QString& input, int& pos) const override;
 
-  int m_max_count;
+private:
+  std::size_t m_max_count;
 };

Source/Core/DolphinQt/TAS/TASCheckBox.cpp

@@ -20,7 +20,7 @@ bool TASCheckBox::GetValue() const
   if (checkState() == Qt::PartiallyChecked)
   {
     const u64 frames_elapsed = Movie::GetCurrentFrame() - m_frame_turbo_started;
-    return frames_elapsed % m_turbo_total_frames < m_turbo_press_frames;
+    return static_cast<int>(frames_elapsed % m_turbo_total_frames) < m_turbo_press_frames;
   }
 
   return isChecked();

Source/Core/InputCommon/ImageOperations.h

@@ -32,13 +32,13 @@ struct ImagePixelData
 {
   ImagePixelData() = default;
 
-  explicit ImagePixelData(std::vector<Pixel> image_pixels, u32 width, u32 height)
+  explicit ImagePixelData(std::vector<Pixel> image_pixels, u32 width_, u32 height_)
-      : pixels(std::move(image_pixels)), width(width), height(height)
+      : pixels(std::move(image_pixels)), width(width_), height(height_)
   {
   }
 
-  explicit ImagePixelData(u32 width, u32 height, const Pixel& default_color = Pixel{0, 0, 0, 0})
+  explicit ImagePixelData(u32 width_, u32 height_, const Pixel& default_color = Pixel{0, 0, 0, 0})
-      : pixels(width * height, default_color), width(width), height(height)
+      : pixels(width_ * height_, default_color), width(width_), height(height_)
   {
   }
   std::vector<Pixel> pixels;