From 1f1a02151ee9bbc18caeb03f54cf06db5fdb963a Mon Sep 17 00:00:00 2001 From: Lioncash Date: Wed, 27 Nov 2019 22:49:23 -0500 Subject: [PATCH 1/2] GCMemcard: Replace ByteSwap with std::swap There's already a standard library function that does what this function is doing. --- Source/Core/Core/HW/GCMemcard/GCMemcard.cpp | 54 +++++++++------------ Source/Core/Core/HW/GCMemcard/GCMemcard.h | 1 - 2 files changed, 24 insertions(+), 31 deletions(-) diff --git a/Source/Core/Core/HW/GCMemcard/GCMemcard.cpp b/Source/Core/Core/HW/GCMemcard/GCMemcard.cpp index 75240765ca..8d499832fa 100644 --- a/Source/Core/Core/HW/GCMemcard/GCMemcard.cpp +++ b/Source/Core/Core/HW/GCMemcard/GCMemcard.cpp @@ -8,6 +8,7 @@ #include #include #include +#include #include #include "Common/BitUtils.h" @@ -20,13 +21,6 @@ #include "Common/StringUtil.h" #include "Common/Swap.h" -static void ByteSwap(u8* valueA, u8* valueB) -{ - u8 tmp = *valueA; - *valueA = *valueB; - *valueB = tmp; -} - static constexpr std::optional BytesToMegabits(u64 bytes) { const u64 factor = ((1024 * 1024) / 8); @@ -1148,38 +1142,38 @@ void GCMemcard::Gcs_SavConvert(DEntry& tempDEntry, int saveType, u64 length) // 0x34 and 0x35, 0x36 and 0x37, 0x38 and 0x39, 0x3A and 0x3B, // 0x3C and 0x3D,0x3E and 0x3F. // It seems that sav files also swap the banner/icon flags... - ByteSwap(&tempDEntry.m_unused_1, &tempDEntry.m_banner_and_icon_flags); + std::swap(tempDEntry.m_unused_1, tempDEntry.m_banner_and_icon_flags); std::array tmp; - memcpy(tmp.data(), &tempDEntry.m_image_offset, 4); - ByteSwap(&tmp[0], &tmp[1]); - ByteSwap(&tmp[2], &tmp[3]); - memcpy(&tempDEntry.m_image_offset, tmp.data(), 4); + std::memcpy(tmp.data(), &tempDEntry.m_image_offset, 4); + std::swap(tmp[0], tmp[1]); + std::swap(tmp[2], tmp[3]); + std::memcpy(&tempDEntry.m_image_offset, tmp.data(), 4); - memcpy(tmp.data(), &tempDEntry.m_icon_format, 2); - ByteSwap(&tmp[0], &tmp[1]); - memcpy(&tempDEntry.m_icon_format, tmp.data(), 2); + std::memcpy(tmp.data(), &tempDEntry.m_icon_format, 2); + std::swap(tmp[0], tmp[1]); + std::memcpy(&tempDEntry.m_icon_format, tmp.data(), 2); - memcpy(tmp.data(), &tempDEntry.m_animation_speed, 2); - ByteSwap(&tmp[0], &tmp[1]); - memcpy(&tempDEntry.m_animation_speed, tmp.data(), 2); + std::memcpy(tmp.data(), &tempDEntry.m_animation_speed, 2); + std::swap(tmp[0], tmp[1]); + std::memcpy(&tempDEntry.m_animation_speed, tmp.data(), 2); - ByteSwap(&tempDEntry.m_file_permissions, &tempDEntry.m_copy_counter); + std::swap(tempDEntry.m_file_permissions, tempDEntry.m_copy_counter); - memcpy(tmp.data(), &tempDEntry.m_first_block, 2); - ByteSwap(&tmp[0], &tmp[1]); - memcpy(&tempDEntry.m_first_block, tmp.data(), 2); + std::memcpy(tmp.data(), &tempDEntry.m_first_block, 2); + std::swap(tmp[0], tmp[1]); + std::memcpy(&tempDEntry.m_first_block, tmp.data(), 2); - memcpy(tmp.data(), &tempDEntry.m_block_count, 2); - ByteSwap(&tmp[0], &tmp[1]); - memcpy(&tempDEntry.m_block_count, tmp.data(), 2); + std::memcpy(tmp.data(), &tempDEntry.m_block_count, 2); + std::swap(tmp[0], tmp[1]); + std::memcpy(&tempDEntry.m_block_count, tmp.data(), 2); - ByteSwap(&tempDEntry.m_unused_2[0], &tempDEntry.m_unused_2[1]); + std::swap(tempDEntry.m_unused_2[0], tempDEntry.m_unused_2[1]); - memcpy(tmp.data(), &tempDEntry.m_comments_address, 4); - ByteSwap(&tmp[0], &tmp[1]); - ByteSwap(&tmp[2], &tmp[3]); - memcpy(&tempDEntry.m_comments_address, tmp.data(), 4); + std::memcpy(tmp.data(), &tempDEntry.m_comments_address, 4); + std::swap(tmp[0], tmp[1]); + std::swap(tmp[2], tmp[3]); + std::memcpy(&tempDEntry.m_comments_address, tmp.data(), 4); break; default: break; diff --git a/Source/Core/Core/HW/GCMemcard/GCMemcard.h b/Source/Core/Core/HW/GCMemcard/GCMemcard.h index f268fa658e..9c8083809d 100644 --- a/Source/Core/Core/HW/GCMemcard/GCMemcard.h +++ b/Source/Core/Core/HW/GCMemcard/GCMemcard.h @@ -27,7 +27,6 @@ class IOFile; #define BE64(x) (Common::swap64(x)) #define BE32(x) (Common::swap32(x)) #define BE16(x) (Common::swap16(x)) -#define ArrayByteSwap(a) (ByteSwap(a, a + sizeof(u8))); enum { From e33c366502fe29508d9e5ea7b85a83ee6a6d96af Mon Sep 17 00:00:00 2001 From: Lioncash Date: Wed, 27 Nov 2019 22:54:50 -0500 Subject: [PATCH 2/2] GCMemcard: Remove byteswapping macros We can just specify the functions directly instead of relying on preprocessor textual replacement. --- .../Core/Core/HW/EXI/EXI_DeviceMemoryCard.cpp | 4 +++- Source/Core/Core/HW/GCMemcard/GCMemcard.cpp | 19 +++++++-------- Source/Core/Core/HW/GCMemcard/GCMemcard.h | 4 ---- .../Core/HW/GCMemcard/GCMemcardDirectory.cpp | 24 +++++++++---------- 4 files changed, 24 insertions(+), 27 deletions(-) diff --git a/Source/Core/Core/HW/EXI/EXI_DeviceMemoryCard.cpp b/Source/Core/Core/HW/EXI/EXI_DeviceMemoryCard.cpp index 7e004ddc92..65c35b12b4 100644 --- a/Source/Core/Core/HW/EXI/EXI_DeviceMemoryCard.cpp +++ b/Source/Core/Core/HW/EXI/EXI_DeviceMemoryCard.cpp @@ -166,7 +166,9 @@ void CEXIMemoryCard::SetupGciFolder(u16 sizeMb) u32 CurrentGameId = 0; if (game_id.length() >= 4 && game_id != "00000000" && SConfig::GetInstance().GetTitleID() != Titles::SYSTEM_MENU) - CurrentGameId = BE32((u8*)game_id.c_str()); + { + CurrentGameId = Common::swap32(reinterpret_cast(game_id.c_str())); + } const bool shift_jis = region == DiscIO::Region::NTSC_J; diff --git a/Source/Core/Core/HW/GCMemcard/GCMemcard.cpp b/Source/Core/Core/HW/GCMemcard/GCMemcard.cpp index 8d499832fa..0f9588f0c8 100644 --- a/Source/Core/Core/HW/GCMemcard/GCMemcard.cpp +++ b/Source/Core/Core/HW/GCMemcard/GCMemcard.cpp @@ -815,9 +815,8 @@ GCMemcardGetSaveDataRetVal GCMemcard::GetSaveData(u8 index, std::vector> 16); - *(u16*)&FileBuffer[3].m_block[0x1580] = BE16(BE32(serial2) >> 16); - *(u16*)&FileBuffer[1].m_block[0x0060] = BE16(BE32(serial1) & 0xFFFF); - *(u16*)&FileBuffer[1].m_block[0x0200] = BE16(BE32(serial2) & 0xFFFF); + *(u16*)&FileBuffer[1].m_block[0x0066] = Common::swap16(u16(Common::swap32(serial1) >> 16)); + *(u16*)&FileBuffer[3].m_block[0x1580] = Common::swap16(u16(Common::swap32(serial2) >> 16)); + *(u16*)&FileBuffer[1].m_block[0x0060] = Common::swap16(u16(Common::swap32(serial1) & 0xFFFF)); + *(u16*)&FileBuffer[1].m_block[0x0200] = Common::swap16(u16(Common::swap32(serial2) & 0xFFFF)); // calc 16-bit checksum for (i = 0x02; i < 0x8000; i++) @@ -1448,7 +1447,7 @@ s32 GCMemcard::FZEROGX_MakeSaveGameValid(const Header& cardheader, const DEntry& } // set new checksum - *(u16*)&FileBuffer[0].m_block[0x00] = BE16(~chksum); + *(u16*)&FileBuffer[0].m_block[0x00] = Common::swap16(u16(~chksum)); return 1; } @@ -1520,7 +1519,7 @@ s32 GCMemcard::PSO_MakeSaveGameValid(const Header& cardheader, const DEntry& dir } // set new checksum - *(u32*)&FileBuffer[1].m_block[0x0048] = BE32(chksum ^ 0xFFFFFFFF); + *(u32*)&FileBuffer[1].m_block[0x0048] = Common::swap32(chksum ^ 0xFFFFFFFF); return 1; } @@ -1639,7 +1638,7 @@ Directory::Directory() { memset(this, 0xFF, BLOCK_SIZE); m_update_counter = 0; - m_checksum = BE16(0xF003); + m_checksum = Common::swap16(0xF003); m_checksum_inv = 0; } diff --git a/Source/Core/Core/HW/GCMemcard/GCMemcard.h b/Source/Core/Core/HW/GCMemcard/GCMemcard.h index 9c8083809d..e650429b81 100644 --- a/Source/Core/Core/HW/GCMemcard/GCMemcard.h +++ b/Source/Core/Core/HW/GCMemcard/GCMemcard.h @@ -24,10 +24,6 @@ namespace File class IOFile; } -#define BE64(x) (Common::swap64(x)) -#define BE32(x) (Common::swap32(x)) -#define BE16(x) (Common::swap16(x)) - enum { SLOT_A = 0, diff --git a/Source/Core/Core/HW/GCMemcard/GCMemcardDirectory.cpp b/Source/Core/Core/HW/GCMemcard/GCMemcardDirectory.cpp index 9210f00d99..4153c7197a 100644 --- a/Source/Core/Core/HW/GCMemcard/GCMemcardDirectory.cpp +++ b/Source/Core/Core/HW/GCMemcard/GCMemcardDirectory.cpp @@ -102,7 +102,7 @@ std::vector GCMemcardDirectory::GetFileNamesForGameID(const std::st u32 game_code = 0; if (game_id.length() >= 4 && game_id != "00000000") - game_code = BE32(reinterpret_cast(game_id.c_str())); + game_code = Common::swap32(reinterpret_cast(game_id.c_str())); std::vector loaded_saves; for (const std::string& file_name : Common::DoFileSearch({directory}, {".gci"})) @@ -136,7 +136,7 @@ std::vector GCMemcardDirectory::GetFileNamesForGameID(const std::st // card (see above method), but since we're only loading the saves for one GameID here, we're // definitely not going to run out of space. - if (game_code == BE32(gci.m_gci_header.m_gamecode.data())) + if (game_code == Common::swap32(gci.m_gci_header.m_gamecode.data())) { loaded_saves.push_back(gci_filename); filenames.push_back(file_name); @@ -175,7 +175,7 @@ GCMemcardDirectory::GCMemcardDirectory(const std::string& directory, int slot, u continue; } - if (m_game_id == BE32(gci.m_gci_header.m_gamecode.data())) + if (m_game_id == Common::swap32(gci.m_gci_header.m_gamecode.data())) gci_current_game.emplace_back(std::move(gci)); else if (!current_game_only) gci_other_games.emplace_back(std::move(gci)); @@ -443,7 +443,7 @@ inline void GCMemcardDirectory::SyncSaves() if (current->m_dir_entries[i].m_gamecode != DEntry::UNINITIALIZED_GAMECODE) { INFO_LOG(EXPANSIONINTERFACE, "Syncing save 0x%x", - BE32(current->m_dir_entries[i].m_gamecode.data())); + Common::swap32(current->m_dir_entries[i].m_gamecode.data())); bool added = false; while (i >= m_saves.size()) { @@ -456,16 +456,16 @@ inline void GCMemcardDirectory::SyncSaves() memcmp((u8*)&(m_saves[i].m_gci_header), (u8*)&(current->m_dir_entries[i]), DENTRY_SIZE)) { m_saves[i].m_dirty = true; - u32 gamecode = BE32(m_saves[i].m_gci_header.m_gamecode.data()); - u32 new_gamecode = BE32(current->m_dir_entries[i].m_gamecode.data()); - u32 old_start = m_saves[i].m_gci_header.m_first_block; - u32 new_start = current->m_dir_entries[i].m_first_block; + const u32 gamecode = Common::swap32(m_saves[i].m_gci_header.m_gamecode.data()); + const u32 new_gamecode = Common::swap32(current->m_dir_entries[i].m_gamecode.data()); + const u32 old_start = m_saves[i].m_gci_header.m_first_block; + const u32 new_start = current->m_dir_entries[i].m_first_block; if ((gamecode != 0xFFFFFFFF) && (gamecode != new_gamecode)) { PanicAlertT("Game overwrote with another games save. Data corruption ahead 0x%x, 0x%x", - BE32(m_saves[i].m_gci_header.m_gamecode.data()), - BE32(current->m_dir_entries[i].m_gamecode.data())); + Common::swap32(m_saves[i].m_gci_header.m_gamecode.data()), + Common::swap32(current->m_dir_entries[i].m_gamecode.data())); } memcpy((u8*)&(m_saves[i].m_gci_header), (u8*)&(current->m_dir_entries[i]), DENTRY_SIZE); if (old_start != new_start) @@ -483,7 +483,7 @@ inline void GCMemcardDirectory::SyncSaves() else if ((i < m_saves.size()) && (*(u32*)&(m_saves[i].m_gci_header) != 0xFFFFFFFF)) { INFO_LOG(EXPANSIONINTERFACE, "Clearing and/or deleting save 0x%x", - BE32(m_saves[i].m_gci_header.m_gamecode.data())); + Common::swap32(m_saves[i].m_gci_header.m_gamecode.data())); m_saves[i].m_gci_header.m_gamecode = DEntry::UNINITIALIZED_GAMECODE; m_saves[i].m_save_data.clear(); m_saves[i].m_used_blocks.clear(); @@ -660,7 +660,7 @@ void GCMemcardDirectory::FlushToFile() // simultaneously // this ensures that the save data for all of the current games gci files are stored in the // savestate - u32 gamecode = BE32(m_saves[i].m_gci_header.m_gamecode.data()); + const u32 gamecode = Common::swap32(m_saves[i].m_gci_header.m_gamecode.data()); if (gamecode != m_game_id && gamecode != 0xFFFFFFFF && !m_saves[i].m_save_data.empty()) { INFO_LOG(EXPANSIONINTERFACE, "Flushing savedata to disk for %s",