Merge pull request #9037 from shuffle2/code-cleanup

Code cleanup

Source/Core/Common/Assert.h

@@ -9,28 +9,6 @@
 #include "Common/Logging/Log.h"
 #include "Common/MsgHandler.h"
 
-#ifdef _WIN32
-#define ASSERT_MSG(_t_, _a_, _fmt_, ...)                                                           \
-  do                                                                                               \
-  {                                                                                                \
-    if (!(_a_))                                                                                    \
-    {                                                                                              \
-      if (!PanicYesNo(_fmt_, __VA_ARGS__))                                                         \
-        Crash();                                                                                   \
-    }                                                                                              \
-  } while (0)
-
-#define DEBUG_ASSERT_MSG(_t_, _a_, _msg_, ...)                                                     \
-  do                                                                                               \
-  {                                                                                                \
-    if (MAX_LOGLEVEL >= Common::Log::LOG_LEVELS::LDEBUG && !(_a_))                                 \
-    {                                                                                              \
-      ERROR_LOG(_t_, _msg_, __VA_ARGS__);                                                          \
-      if (!PanicYesNo(_msg_, __VA_ARGS__))                                                         \
-        Crash();                                                                                   \
-    }                                                                                              \
-  } while (0)
-#else
 #define ASSERT_MSG(_t_, _a_, _fmt_, ...)                                                           \
   do                                                                                               \
   {                                                                                                \
@@ -44,14 +22,16 @@
 #define DEBUG_ASSERT_MSG(_t_, _a_, _msg_, ...)                                                     \
   do                                                                                               \
   {                                                                                                \
-    if (MAX_LOGLEVEL >= Common::Log::LOG_LEVELS::LDEBUG && !(_a_))                                 \
+    if constexpr (MAX_LOGLEVEL >= Common::Log::LOG_LEVELS::LDEBUG)                                 \
     {                                                                                              \
-      ERROR_LOG(_t_, _msg_, ##__VA_ARGS__);                                                        \
-      if (!PanicYesNo(_msg_, ##__VA_ARGS__))                                                       \
-        Crash();                                                                                   \
+      if (!(_a_))                                                                                  \
+      {                                                                                            \
+        ERROR_LOG(_t_, _msg_, ##__VA_ARGS__);                                                      \
+        if (!PanicYesNo(_msg_, ##__VA_ARGS__))                                                     \
+          Crash();                                                                                 \
+      }                                                                                            \
     }                                                                                              \
   } while (0)
-#endif
 
 #define ASSERT(_a_)                                                                                \
   do                                                                                               \
@@ -64,6 +44,6 @@
 #define DEBUG_ASSERT(_a_)                                                                          \
   do                                                                                               \
   {                                                                                                \
-    if (MAX_LOGLEVEL >= Common::Log::LOG_LEVELS::LDEBUG)                                           \
+    if constexpr (MAX_LOGLEVEL >= Common::Log::LOG_LEVELS::LDEBUG)                                 \
       ASSERT(_a_);                                                                                 \
   } while (0)

Source/Core/Common/CompatPatches.cpp

@@ -183,7 +183,7 @@ static bool GetModuleVersion(const wchar_t* name, Version* version)
   if (!data_len)
     return false;
   std::vector<u8> block(data_len);
-  if (!GetFileVersionInfoW(path->c_str(), handle, data_len, block.data()))
+  if (!GetFileVersionInfoW(path->c_str(), 0, data_len, block.data()))
     return false;
   void* buf;
   UINT buf_len;

Source/Core/Common/FileSearch.cpp

@@ -98,7 +98,7 @@ std::vector<std::string> DoFileSearch(const std::vector<std::string>& directorie
   };
   for (const auto& directory : directories)
   {
-    const fs::path directory_path = StringToPath(directory);
+    fs::path directory_path = StringToPath(directory);
     if (fs::is_directory(directory_path))  // Can't create iterators for non-existant directories
     {
       if (recursive)
@@ -125,9 +125,11 @@ std::vector<std::string> DoFileSearch(const std::vector<std::string>& directorie
   // std::filesystem uses the OS separator.
   constexpr fs::path::value_type os_separator = fs::path::preferred_separator;
   static_assert(os_separator == DIR_SEP_CHR || os_separator == '\\', "Unsupported path separator");
-  if (os_separator != DIR_SEP_CHR)
+  if constexpr (os_separator != DIR_SEP_CHR)
+  {
     for (auto& path : result)
       std::replace(path.begin(), path.end(), '\\', DIR_SEP_CHR);
+  }
 
   return result;
 }

Source/Core/Common/FileUtil.cpp

@@ -600,7 +600,7 @@ std::string GetCurrentDir()
   if (!dir)
   {
     ERROR_LOG(COMMON, "GetCurrentDirectory failed: %s", LastStrerrorString().c_str());
-    return nullptr;
+    return "";
   }
   std::string strDir = dir;
   free(dir);
@@ -621,10 +621,15 @@ std::string CreateTempDir()
     return "";
 
   GUID guid;
-  CoCreateGuid(&guid);
-  TCHAR tguid[40];
-  StringFromGUID2(guid, tguid, 39);
-  tguid[39] = 0;
+  if (FAILED(CoCreateGuid(&guid)))
+  {
+    return "";
+  }
+  OLECHAR tguid[40]{};
+  if (!StringFromGUID2(guid, tguid, _countof(tguid)))
+  {
+    return "";
+  }
   std::string dir = TStrToUTF8(temp) + "/" + TStrToUTF8(tguid);
   if (!CreateDir(dir))
     return "";

Source/Core/Common/GekkoDisassembler.cpp

@@ -1470,7 +1470,7 @@ u32* GekkoDisassembler::DoDisassembly(bool big_endian)
     break;
 
   case 30:
-    switch (in & 0x1c)
+    switch ((in >> 2) & 0x7)
     {
     case 0:
       rld(in, "icl", 0);  // rldicl

Source/Core/Common/MsgHandler.h

@@ -32,39 +32,6 @@ bool MsgAlert(bool yes_no, MsgType style, const char* format, ...)
 void SetEnableAlert(bool enable);
 }  // namespace Common
 
-#if defined(_WIN32) && (!defined(_MSVC_TRADITIONAL) || _MSVC_TRADITIONAL == 1)
-#define SuccessAlert(format, ...)                                                                  \
-  Common::MsgAlert(false, Common::MsgType::Information, format, __VA_ARGS__)
-
-#define PanicAlert(format, ...)                                                                    \
-  Common::MsgAlert(false, Common::MsgType::Warning, format, __VA_ARGS__)
-
-#define PanicYesNo(format, ...)                                                                    \
-  Common::MsgAlert(true, Common::MsgType::Warning, format, __VA_ARGS__)
-
-#define AskYesNo(format, ...) Common::MsgAlert(true, Common::MsgType::Question, format, __VA_ARGS__)
-
-#define CriticalAlert(format, ...)                                                                 \
-  Common::MsgAlert(false, Common::MsgType::Critical, format, __VA_ARGS__)
-
-// Use these macros (that do the same thing) if the message should be translated.
-
-#define SuccessAlertT(format, ...)                                                                 \
-  Common::MsgAlert(false, Common::MsgType::Information, format, __VA_ARGS__)
-
-#define PanicAlertT(format, ...)                                                                   \
-  Common::MsgAlert(false, Common::MsgType::Warning, format, __VA_ARGS__)
-
-#define PanicYesNoT(format, ...)                                                                   \
-  Common::MsgAlert(true, Common::MsgType::Warning, format, __VA_ARGS__)
-
-#define AskYesNoT(format, ...)                                                                     \
-  Common::MsgAlert(true, Common::MsgType::Question, format, __VA_ARGS__)
-
-#define CriticalAlertT(format, ...)                                                                \
-  Common::MsgAlert(false, Common::MsgType::Critical, format, __VA_ARGS__)
-
-#else
 #define SuccessAlert(format, ...)                                                                  \
   Common::MsgAlert(false, Common::MsgType::Information, format, ##__VA_ARGS__)
 
@@ -95,4 +62,3 @@ void SetEnableAlert(bool enable);
 
 #define CriticalAlertT(format, ...)                                                                \
   Common::MsgAlert(false, Common::MsgType::Critical, format, ##__VA_ARGS__)
-#endif

Source/Core/Common/SDCardUtil.cpp

@@ -247,7 +247,7 @@ bool SDCardCreate(u64 disk_size /*in MB*/, const std::string& filename)
   if (!write_sector(file, s_fsinfo_sector))
     goto FailWrite;
 
-  if (BACKUP_BOOT_SECTOR > 0)
+  if constexpr (BACKUP_BOOT_SECTOR > 0)
   {
     if (!write_empty(file, BACKUP_BOOT_SECTOR - 2))
       goto FailWrite;

Source/Core/Common/Semaphore.h

@@ -6,9 +6,6 @@
 
 #ifdef _WIN32
 
-#ifndef WIN32_LEAN_AND_MEAN
-#define WIN32_LEAN_AND_MEAN
-#endif
 #include <Windows.h>
 
 namespace Common

Source/Core/Common/Timer.cpp

@@ -254,7 +254,7 @@ std::string Timer::GetDateTimeFormatted(double time)
 
 #ifdef _WIN32
   wchar_t tmp[32] = {};
-  wcsftime(tmp, sizeof(tmp), L"%x %X", localTime);
+  wcsftime(tmp, std::size(tmp), L"%x %X", localTime);
   return WStringToUTF8(tmp);
 #else
   char tmp[32] = {};

Source/Core/Core/HW/EXI/BBA/TAP_Win32.cpp

@@ -35,15 +35,16 @@ bool IsTAPDevice(const TCHAR* guid)
     TCHAR net_cfg_instance_id[256];
     DWORD data_type;
 
-    len = sizeof(enum_name);
+    len = _countof(enum_name);
     status = RegEnumKeyEx(netcard_key, i, enum_name, &len, nullptr, nullptr, nullptr, nullptr);
 
     if (status == ERROR_NO_MORE_ITEMS)
       break;
     else if (status != ERROR_SUCCESS)
       return false;
 
-    _sntprintf(unit_string, sizeof(unit_string), _T("%s\\%s"), ADAPTER_KEY, enum_name);
+    _sntprintf(unit_string, _countof(unit_string), _T("%s\\%s"), ADAPTER_KEY, enum_name);
+    unit_string[_countof(unit_string) - 1] = _T('\0');
 
     status = RegOpenKeyEx(HKEY_LOCAL_MACHINE, unit_string, 0, KEY_READ, &unit_key);
 
@@ -110,14 +111,15 @@ bool GetGUIDs(std::vector<std::basic_string<TCHAR>>& guids)
     DWORD name_type;
     const TCHAR name_string[] = _T("Name");
 
-    len = sizeof(enum_name);
+    len = _countof(enum_name);
     status = RegEnumKeyEx(control_net_key, i, enum_name, &len, nullptr, nullptr, nullptr, nullptr);
 
     if (status != ERROR_SUCCESS)
       continue;
 
-    _sntprintf(connection_string, sizeof(connection_string), _T("%s\\%s\\Connection"),
+    _sntprintf(connection_string, _countof(connection_string), _T("%s\\%s\\Connection"),
                NETWORK_CONNECTIONS_KEY, enum_name);
+    connection_string[_countof(connection_string) - 1] = _T('\0');
 
     status = RegOpenKeyEx(HKEY_LOCAL_MACHINE, connection_string, 0, KEY_READ, &connection_key);
 
@@ -196,7 +198,7 @@ bool CEXIETHERNET::TAPNetworkInterface::Activate()
   }
 
   /* get driver version info */
-  ULONG info[3];
+  ULONG info[3]{};
   if (DeviceIoControl(mHAdapter, TAP_IOCTL_GET_VERSION, &info, sizeof(info), &info, sizeof(info),
                       &len, nullptr))
   {

Source/Core/Core/HW/WiimoteCommon/WiimoteHid.h

@@ -54,7 +54,7 @@ struct TypedHIDInputData
 
   T data;
 
-  static_assert(std::is_pod<T>());
+  static_assert(std::is_standard_layout_v<T> && std::is_trivially_copyable_v<T>);
 
   u8* GetData() { return reinterpret_cast<u8*>(this); }
   const u8* GetData() const { return reinterpret_cast<const u8*>(this); }

Source/Core/Core/HW/WiimoteEmu/I2CBus.h

@@ -26,7 +26,7 @@ class I2CSlave
   template <typename T>
   static int RawRead(T* reg_data, u8 addr, int count, u8* data_out)
   {
-    static_assert(std::is_pod<T>::value);
+    static_assert(std::is_standard_layout_v<T> && std::is_trivially_copyable_v<T>);
     static_assert(0x100 == sizeof(T));
 
     // TODO: addr wraps around after 0xff
@@ -42,7 +42,7 @@ class I2CSlave
   template <typename T>
   static int RawWrite(T* reg_data, u8 addr, int count, const u8* data_in)
   {
-    static_assert(std::is_pod<T>::value);
+    static_assert(std::is_standard_layout_v<T> && std::is_trivially_copyable_v<T>);
     static_assert(0x100 == sizeof(T));
 
     // TODO: addr wraps around after 0xff

Source/Core/Core/HW/WiimoteReal/IOWin.cpp

@@ -418,6 +418,10 @@ int WriteToHandle(HANDLE& dev_handle, WinWriteMethod& method, const u8* buf, siz
 {
   OVERLAPPED hid_overlap_write = OVERLAPPED();
   hid_overlap_write.hEvent = CreateEvent(nullptr, true, false, nullptr);
+  if (!hid_overlap_write.hEvent)
+  {
+    return 0;
+  }
 
   DWORD written = 0;
   IOWrite(dev_handle, hid_overlap_write, method, buf, size, &written);
@@ -431,6 +435,10 @@ int ReadFromHandle(HANDLE& dev_handle, u8* buf)
 {
   OVERLAPPED hid_overlap_read = OVERLAPPED();
   hid_overlap_read.hEvent = CreateEvent(nullptr, true, false, nullptr);
+  if (!hid_overlap_read.hEvent)
+  {
+    return 0;
+  }
   const int read = IORead(dev_handle, hid_overlap_read, buf, 1);
   CloseHandle(hid_overlap_read.hEvent);
   return read;
@@ -533,15 +541,15 @@ void WiimoteScannerWindows::FindWiimotes(std::vector<Wiimote*>& found_wiimotes,
 
   SP_DEVICE_INTERFACE_DATA device_data = {};
   device_data.cbSize = sizeof(device_data);
-  PSP_DEVICE_INTERFACE_DETAIL_DATA detail_data = nullptr;
 
   for (int index = 0;
        SetupDiEnumDeviceInterfaces(device_info, nullptr, &device_id, index, &device_data); ++index)
   {
     // Get the size of the data block required
     DWORD len;
     SetupDiGetDeviceInterfaceDetail(device_info, &device_data, nullptr, 0, &len, nullptr);
-    detail_data = (PSP_DEVICE_INTERFACE_DETAIL_DATA)malloc(len);
+    auto detail_data_buf = std::make_unique<u8[]>(len);
+    auto detail_data = reinterpret_cast<PSP_DEVICE_INTERFACE_DETAIL_DATA>(detail_data_buf.get());
     detail_data->cbSize = sizeof(SP_DEVICE_INTERFACE_DETAIL_DATA);
 
     SP_DEVINFO_DATA device_info_data = {};
@@ -558,7 +566,6 @@ void WiimoteScannerWindows::FindWiimotes(std::vector<Wiimote*>& found_wiimotes,
 
       if (!IsNewWiimote(WStringToUTF8(device_path)) || !IsWiimote(device_path, write_method))
       {
-        free(detail_data);
         continue;
       }
 
@@ -568,8 +575,6 @@ void WiimoteScannerWindows::FindWiimotes(std::vector<Wiimote*>& found_wiimotes,
       else
         found_wiimotes.push_back(wiimote);
     }
-
-    free(detail_data);
   }
 
   SetupDiDestroyDeviceInfoList(device_info);

Source/Core/Core/IOS/ES/Formats.cpp

@@ -621,7 +621,7 @@ UIDSys::UIDSys(std::shared_ptr<HLE::FS::FileSystem> fs) : m_fs{fs}
   {
     while (true)
     {
-      const std::pair<u32, u64> entry = ReadUidSysEntry(*file);
+      std::pair<u32, u64> entry = ReadUidSysEntry(*file);
       if (!entry.first && !entry.second)
         break;
 
@@ -766,7 +766,7 @@ std::map<std::string, CertReader> ParseCertChain(const std::vector<u8>& chain)
       return certs;
 
     processed += cert_reader.GetBytes().size();
-    const std::string name = cert_reader.GetName();
+    std::string name = cert_reader.GetName();
     certs.emplace(std::move(name), std::move(cert_reader));
   }
   return certs;

Source/Core/Core/IOS/IOS.cpp

@@ -824,7 +824,8 @@ void Init()
     if (!s_ios)
       return;
 
-    auto device = static_cast<Device::SDIOSlot0*>(s_ios->GetDeviceByName("/dev/sdio/slot0").get());
+    auto sdio_slot0 = s_ios->GetDeviceByName("/dev/sdio/slot0");
+    auto device = static_cast<Device::SDIOSlot0*>(sdio_slot0.get());
     if (device)
       device->EventNotify();
   });

Source/Core/Core/IOS/Network/IP/Top.cpp

@@ -86,13 +86,12 @@ static constexpr u32 inet_addr(u8 a, u8 b, u8 c, u8 d)
 
 static int inet_pton(const char* src, unsigned char* dst)
 {
-  int saw_digit, octets;
+  int saw_digit = 0;
+  int octets = 0;
+  unsigned char tmp[4]{};
+  unsigned char* tp = tmp;
   char ch;
-  unsigned char tmp[4], *tp;
 
-  saw_digit = 0;
-  octets = 0;
-  *(tp = tmp) = 0;
   while ((ch = *src++) != '\0')
   {
     if (ch >= '0' && ch <= '9')
@@ -927,8 +926,9 @@ IPCCommandResult NetIPTop::HandleRecvFromRequest(const IOCtlVRequest& request)
 IPCCommandResult NetIPTop::HandleGetAddressInfoRequest(const IOCtlVRequest& request)
 {
   addrinfo hints;
+  const bool hints_valid = request.in_vectors.size() > 2 && request.in_vectors[2].size;
 
-  if (request.in_vectors.size() > 2 && request.in_vectors[2].size)
+  if (hints_valid)
   {
     hints.ai_flags = Memory::Read_U32(request.in_vectors[2].address);
     hints.ai_family = Memory::Read_U32(request.in_vectors[2].address + 0x4);
@@ -959,9 +959,7 @@ IPCCommandResult NetIPTop::HandleGetAddressInfoRequest(const IOCtlVRequest& requ
   }
 
   addrinfo* result = nullptr;
-  int ret = getaddrinfo(
-      pNodeName, pServiceName,
-      (request.in_vectors.size() > 2 && request.in_vectors[2].size) ? &hints : nullptr, &result);
+  int ret = getaddrinfo(pNodeName, pServiceName, hints_valid ? &hints : nullptr, &result);
   u32 addr = request.io_vectors[0].address;
   u32 sockoffset = addr + 0x460;
   if (ret == 0)