From 0052b313d6a5f4749043ce89531035a94e3c813c Mon Sep 17 00:00:00 2001 From: "Admiral H. Curtiss" Date: Sat, 4 May 2019 23:28:52 +0200 Subject: [PATCH 01/12] GCMemcard: Directory: Move code out of header, add some boundary checks, fix naming conventions. --- Source/Core/Core/HW/GCMemcard/GCMemcard.cpp | 23 +++++++++++ Source/Core/Core/HW/GCMemcard/GCMemcard.h | 40 +++++++++++-------- .../Core/HW/GCMemcard/GCMemcardDirectory.cpp | 2 +- 3 files changed, 47 insertions(+), 18 deletions(-) diff --git a/Source/Core/Core/HW/GCMemcard/GCMemcard.cpp b/Source/Core/Core/HW/GCMemcard/GCMemcard.cpp index 9eec76d717..62063f2cc9 100644 --- a/Source/Core/Core/HW/GCMemcard/GCMemcard.cpp +++ b/Source/Core/Core/HW/GCMemcard/GCMemcard.cpp @@ -1387,3 +1387,26 @@ s32 GCMemcard::PSO_MakeSaveGameValid(const Header& cardheader, const DEntry& dir return 1; } + +Directory::Directory() +{ + memset(this, 0xFF, BLOCK_SIZE); + m_update_counter = 0; + m_checksum = BE16(0xF003); + m_checksum_inv = 0; +} + +bool Directory::Replace(const DEntry& entry, size_t index) +{ + if (index >= m_dir_entries.size()) + return false; + + m_dir_entries[index] = entry; + FixChecksums(); + return true; +} + +void Directory::FixChecksums() +{ + calc_checksumsBE((u16*)this, 0xFFE, &m_checksum, &m_checksum_inv); +} diff --git a/Source/Core/Core/HW/GCMemcard/GCMemcard.h b/Source/Core/Core/HW/GCMemcard/GCMemcard.h index 662257715d..6bf6bf48ee 100644 --- a/Source/Core/Core/HW/GCMemcard/GCMemcard.h +++ b/Source/Core/Core/HW/GCMemcard/GCMemcard.h @@ -272,24 +272,30 @@ static_assert(sizeof(DEntry) == DENTRY_SIZE); struct Directory { - std::array m_dir_entries; // 0x0000 Directory Entries (max 127) + // 127 files of 0x40 bytes each + std::array m_dir_entries; + + // 0x3a bytes at 0x1fc0: Unused, always 0xFF std::array m_padding; - Common::BigEndianValue m_update_counter; // 0x1ffa 2 Update Counter - u16 m_checksum; // 0x1ffc 2 Additive Checksum - u16 m_checksum_inv; // 0x1ffe 2 Inverse Checksum - Directory() - { - memset(this, 0xFF, BLOCK_SIZE); - m_update_counter = 0; - m_checksum = BE16(0xF003); - m_checksum_inv = 0; - } - void Replace(DEntry d, int idx) - { - m_dir_entries[idx] = d; - fixChecksums(); - } - void fixChecksums() { calc_checksumsBE((u16*)this, 0xFFE, &m_checksum, &m_checksum_inv); } + + // 2 bytes at 0x1ffa: Update Counter + // TODO: What happens if this overflows? Is there a special case for preferring 0 over max value? + Common::BigEndianValue m_update_counter; + + // 2 bytes at 0x1ffc: Additive Checksum + u16 m_checksum; + + // 2 bytes at 0x1ffe: Inverse Checksum + u16 m_checksum_inv; + + // Constructs an empty Directory block. + Directory(); + + // Replaces the file metadata at the given index (range 0-126) + // with the given DEntry data. + bool Replace(const DEntry& entry, size_t index); + + void FixChecksums(); }; static_assert(sizeof(Directory) == BLOCK_SIZE); diff --git a/Source/Core/Core/HW/GCMemcard/GCMemcardDirectory.cpp b/Source/Core/Core/HW/GCMemcard/GCMemcardDirectory.cpp index 7b5e56678f..0353e9e4b8 100644 --- a/Source/Core/Core/HW/GCMemcard/GCMemcardDirectory.cpp +++ b/Source/Core/Core/HW/GCMemcard/GCMemcardDirectory.cpp @@ -215,7 +215,7 @@ GCMemcardDirectory::GCMemcardDirectory(const std::string& directory, int slot, u } m_loaded_saves.clear(); - m_dir1.fixChecksums(); + m_dir1.FixChecksums(); m_dir2 = m_dir1; m_bat2 = m_bat1; From 4b46f71b3c3ce3710a7bca1d1597d0bf62192d0b Mon Sep 17 00:00:00 2001 From: "Admiral H. Curtiss" Date: Sat, 4 May 2019 23:53:27 +0200 Subject: [PATCH 02/12] GCMemcard: Header: Move code out of header, fix naming conventions. --- Source/Core/Core/HW/GCMemcard/GCMemcard.cpp | 44 ++++++++++++++++++++- Source/Core/Core/HW/GCMemcard/GCMemcard.h | 42 ++------------------ 2 files changed, 46 insertions(+), 40 deletions(-) diff --git a/Source/Core/Core/HW/GCMemcard/GCMemcard.cpp b/Source/Core/Core/HW/GCMemcard/GCMemcard.cpp index 62063f2cc9..e991f90ec4 100644 --- a/Source/Core/Core/HW/GCMemcard/GCMemcard.cpp +++ b/Source/Core/Core/HW/GCMemcard/GCMemcard.cpp @@ -1286,7 +1286,7 @@ s32 GCMemcard::FZEROGX_MakeSaveGameValid(const Header& cardheader, const DEntry& return 0; // get encrypted destination memory card serial numbers - cardheader.CARD_GetSerialNo(&serial1, &serial2); + cardheader.CalculateSerial(&serial1, &serial2); // set new serial numbers *(u16*)&FileBuffer[1].m_block[0x0066] = BE16(BE32(serial1) >> 16); @@ -1352,7 +1352,7 @@ s32 GCMemcard::PSO_MakeSaveGameValid(const Header& cardheader, const DEntry& dir } // get encrypted destination memory card serial numbers - cardheader.CARD_GetSerialNo(&serial1, &serial2); + cardheader.CalculateSerial(&serial1, &serial2); // set new serial numbers *(u32*)&FileBuffer[1].m_block[0x0158] = serial1; @@ -1388,6 +1388,46 @@ s32 GCMemcard::PSO_MakeSaveGameValid(const Header& cardheader, const DEntry& dir return 1; } +Header::Header(int slot, u16 size_mbits, bool shift_jis) +{ + // Nintendo format algorithm. + // Constants are fixed by the GC SDK + // Changing the constants will break memory card support + memset(this, 0xFF, BLOCK_SIZE); + m_size_mb = size_mbits; + m_encoding = shift_jis ? 1 : 0; + u64 rand = Common::Timer::GetLocalTimeSinceJan1970() - ExpansionInterface::CEXIIPL::GC_EPOCH; + m_format_time = rand; + for (int i = 0; i < 12; i++) + { + rand = (((rand * (u64)0x0000000041c64e6dULL) + (u64)0x0000000000003039ULL) >> 16); + m_serial[i] = (u8)(g_SRAM.settings_ex.flash_id[slot][i] + (u32)rand); + rand = (((rand * (u64)0x0000000041c64e6dULL) + (u64)0x0000000000003039ULL) >> 16); + rand &= (u64)0x0000000000007fffULL; + } + m_sram_bias = g_SRAM.settings.rtc_bias; + m_sram_language = static_cast(g_SRAM.settings.language); + // TODO: determine the purpose of m_unknown_2 + // 1 works for slot A, 0 works for both slot A and slot B + memset(m_unknown_2.data(), 0, + m_unknown_2.size()); // = _viReg[55]; static vu16* const _viReg = (u16*)0xCC002000; + m_device_id = 0; + calc_checksumsBE((u16*)this, 0xFE, &m_checksum, &m_checksum_inv); +} + +void Header::CalculateSerial(u32* serial1, u32* serial2) const +{ + u32 serial[8]; + + for (int i = 0; i < 8; i++) + { + memcpy(&serial[i], (u8*)this + (i * 4), 4); + } + + *serial1 = serial[0] ^ serial[2] ^ serial[4] ^ serial[6]; + *serial2 = serial[1] ^ serial[3] ^ serial[5] ^ serial[7]; +} + Directory::Directory() { memset(this, 0xFF, BLOCK_SIZE); diff --git a/Source/Core/Core/HW/GCMemcard/GCMemcard.h b/Source/Core/Core/HW/GCMemcard/GCMemcard.h index 6bf6bf48ee..13d0d3d4e6 100644 --- a/Source/Core/Core/HW/GCMemcard/GCMemcard.h +++ b/Source/Core/Core/HW/GCMemcard/GCMemcard.h @@ -133,6 +133,7 @@ struct Header std::array m_unused_1; // 2 bytes at 0x01fa: Update Counter (?, probably unused) + // TODO: This seems to be 0xFFFF in all my memory cards, might still be part of m_unused_1. u16 m_update_counter; // 2 bytes at 0x01fc: Additive Checksum @@ -144,45 +145,10 @@ struct Header // 0x1e00 bytes at 0x0200: Unused (0xff) std::array m_unused_2; - void CARD_GetSerialNo(u32* serial1, u32* serial2) const - { - u32 serial[8]; + explicit Header(int slot = 0, u16 size_mbits = MemCard2043Mb, bool shift_jis = false); - for (int i = 0; i < 8; i++) - { - memcpy(&serial[i], (u8*)this + (i * 4), 4); - } - - *serial1 = serial[0] ^ serial[2] ^ serial[4] ^ serial[6]; - *serial2 = serial[1] ^ serial[3] ^ serial[5] ^ serial[7]; - } - - // Nintendo format algorithm. - // Constants are fixed by the GC SDK - // Changing the constants will break memory card support - explicit Header(int slot = 0, u16 sizeMb = MemCard2043Mb, bool shift_jis = false) - { - memset(this, 0xFF, BLOCK_SIZE); - m_size_mb = sizeMb; - m_encoding = shift_jis ? 1 : 0; - u64 rand = Common::Timer::GetLocalTimeSinceJan1970() - ExpansionInterface::CEXIIPL::GC_EPOCH; - m_format_time = rand; - for (int i = 0; i < 12; i++) - { - rand = (((rand * (u64)0x0000000041c64e6dULL) + (u64)0x0000000000003039ULL) >> 16); - m_serial[i] = (u8)(g_SRAM.settings_ex.flash_id[slot][i] + (u32)rand); - rand = (((rand * (u64)0x0000000041c64e6dULL) + (u64)0x0000000000003039ULL) >> 16); - rand &= (u64)0x0000000000007fffULL; - } - m_sram_bias = g_SRAM.settings.rtc_bias; - m_sram_language = static_cast(g_SRAM.settings.language); - // TODO: determine the purpose of m_unknown_2 - // 1 works for slot A, 0 works for both slot A and slot B - memset(m_unknown_2.data(), 0, - m_unknown_2.size()); // = _viReg[55]; static vu16* const _viReg = (u16*)0xCC002000; - m_device_id = 0; - calc_checksumsBE((u16*)this, 0xFE, &m_checksum, &m_checksum_inv); - } + // Calculates the card serial numbers used for encrypting some save files. + void CalculateSerial(u32* serial1, u32* serial2) const; }; static_assert(sizeof(Header) == BLOCK_SIZE); From e4094d9d2d238ec5164290a826c07dce0c9c8966 Mon Sep 17 00:00:00 2001 From: "Admiral H. Curtiss" Date: Sun, 5 May 2019 00:49:47 +0200 Subject: [PATCH 03/12] GCMemcard: BlockAlloc: Move code out of header, fix naming conventions. --- Source/Core/Core/HW/GCMemcard/GCMemcard.cpp | 62 ++++++++++++++++----- Source/Core/Core/HW/GCMemcard/GCMemcard.h | 38 +++---------- 2 files changed, 54 insertions(+), 46 deletions(-) diff --git a/Source/Core/Core/HW/GCMemcard/GCMemcard.cpp b/Source/Core/Core/HW/GCMemcard/GCMemcard.cpp index e991f90ec4..e2187c72b9 100644 --- a/Source/Core/Core/HW/GCMemcard/GCMemcard.cpp +++ b/Source/Core/Core/HW/GCMemcard/GCMemcard.cpp @@ -594,57 +594,89 @@ std::optional GCMemcard::GetDEntry(u8 index) const return GetActiveDirectory().m_dir_entries[index]; } -u16 BlockAlloc::GetNextBlock(u16 Block) const +BlockAlloc::BlockAlloc(u16 size_mbits) { - if ((Block < MC_FST_BLOCKS) || (Block > 4091)) + memset(this, 0, BLOCK_SIZE); + m_free_blocks = (size_mbits * MBIT_TO_BLOCKS) - MC_FST_BLOCKS; + m_last_allocated_block = 4; + FixChecksums(); +} + +u16 BlockAlloc::GetNextBlock(u16 block) const +{ + // FIXME: This is fishy, shouldn't that be in range [5, 4096[? + if ((block < MC_FST_BLOCKS) || (block > 4091)) return 0; - return m_map[Block - MC_FST_BLOCKS]; + return m_map[block - MC_FST_BLOCKS]; } // Parameters and return value are expected as memory card block index, // not BAT index; that is, block 5 is the first file data block. -u16 BlockAlloc::NextFreeBlock(u16 MaxBlock, u16 StartingBlock) const +u16 BlockAlloc::NextFreeBlock(u16 max_block, u16 starting_block) const { if (m_free_blocks > 0) { - StartingBlock = std::clamp(StartingBlock, MC_FST_BLOCKS, BAT_SIZE + MC_FST_BLOCKS); - MaxBlock = std::clamp(MaxBlock, MC_FST_BLOCKS, BAT_SIZE + MC_FST_BLOCKS); - for (u16 i = StartingBlock; i < MaxBlock; ++i) + starting_block = std::clamp(starting_block, MC_FST_BLOCKS, BAT_SIZE + MC_FST_BLOCKS); + max_block = std::clamp(max_block, MC_FST_BLOCKS, BAT_SIZE + MC_FST_BLOCKS); + for (u16 i = starting_block; i < max_block; ++i) if (m_map[i - MC_FST_BLOCKS] == 0) return i; - for (u16 i = MC_FST_BLOCKS; i < StartingBlock; ++i) + for (u16 i = MC_FST_BLOCKS; i < starting_block; ++i) if (m_map[i - MC_FST_BLOCKS] == 0) return i; } return 0xFFFF; } -bool BlockAlloc::ClearBlocks(u16 FirstBlock, u16 BlockCount) +bool BlockAlloc::ClearBlocks(u16 starting_block, u16 block_count) { std::vector blocks; - while (FirstBlock != 0xFFFF && FirstBlock != 0) + while (starting_block != 0xFFFF && starting_block != 0) { - blocks.push_back(FirstBlock); - FirstBlock = GetNextBlock(FirstBlock); + blocks.push_back(starting_block); + starting_block = GetNextBlock(starting_block); } - if (FirstBlock > 0) + if (starting_block > 0) { size_t length = blocks.size(); - if (length != BlockCount) + if (length != block_count) { return false; } for (unsigned int i = 0; i < length; ++i) m_map[blocks.at(i) - MC_FST_BLOCKS] = 0; - m_free_blocks = m_free_blocks + BlockCount; + m_free_blocks = m_free_blocks + block_count; return true; } return false; } +void BlockAlloc::FixChecksums() +{ + calc_checksumsBE((u16*)&m_update_counter, 0xFFE, &m_checksum, &m_checksum_inv); +} + +u16 BlockAlloc::AssignBlocksContiguous(u16 length) +{ + u16 starting = m_last_allocated_block + 1; + if (length > m_free_blocks) + return 0xFFFF; + u16 current = starting; + while ((current - starting + 1) < length) + { + m_map[current - 5] = current + 1; + current++; + } + m_map[current - 5] = 0xFFFF; + m_last_allocated_block = current; + m_free_blocks = m_free_blocks - length; + FixChecksums(); + return starting; +} + u32 GCMemcard::GetSaveData(u8 index, std::vector& Blocks) const { if (!m_valid) diff --git a/Source/Core/Core/HW/GCMemcard/GCMemcard.h b/Source/Core/Core/HW/GCMemcard/GCMemcard.h index 13d0d3d4e6..30ef094ddf 100644 --- a/Source/Core/Core/HW/GCMemcard/GCMemcard.h +++ b/Source/Core/Core/HW/GCMemcard/GCMemcard.h @@ -285,37 +285,13 @@ struct BlockAlloc // 0x1ff8 bytes at 0x000a: Map of allocated Blocks std::array, BAT_SIZE> m_map; - u16 GetNextBlock(u16 Block) const; - u16 NextFreeBlock(u16 MaxBlock, u16 StartingBlock = MC_FST_BLOCKS) const; - bool ClearBlocks(u16 StartingBlock, u16 Length); - void fixChecksums() - { - calc_checksumsBE((u16*)&m_update_counter, 0xFFE, &m_checksum, &m_checksum_inv); - } - explicit BlockAlloc(u16 sizeMb = MemCard2043Mb) - { - memset(this, 0, BLOCK_SIZE); - m_free_blocks = (sizeMb * MBIT_TO_BLOCKS) - MC_FST_BLOCKS; - m_last_allocated_block = 4; - fixChecksums(); - } - u16 AssignBlocksContiguous(u16 length) - { - u16 starting = m_last_allocated_block + 1; - if (length > m_free_blocks) - return 0xFFFF; - u16 current = starting; - while ((current - starting + 1) < length) - { - m_map[current - 5] = current + 1; - current++; - } - m_map[current - 5] = 0xFFFF; - m_last_allocated_block = current; - m_free_blocks = m_free_blocks - length; - fixChecksums(); - return starting; - } + explicit BlockAlloc(u16 size_mbits = MemCard2043Mb); + + u16 GetNextBlock(u16 block) const; + u16 NextFreeBlock(u16 max_block, u16 starting_block = MC_FST_BLOCKS) const; + bool ClearBlocks(u16 starting_block, u16 block_count); + void FixChecksums(); + u16 AssignBlocksContiguous(u16 length); }; static_assert(sizeof(BlockAlloc) == BLOCK_SIZE); #pragma pack(pop) From 17da22a84fd0da2bb303806459aad558f96ca90b Mon Sep 17 00:00:00 2001 From: "Admiral H. Curtiss" Date: Sun, 21 Apr 2019 17:37:03 +0200 Subject: [PATCH 04/12] GCMemcard: GCMBlock: Move code out of header. --- Source/Core/Core/HW/GCMemcard/GCMemcard.cpp | 10 ++++++++++ Source/Core/Core/HW/GCMemcard/GCMemcard.h | 4 ++-- 2 files changed, 12 insertions(+), 2 deletions(-) diff --git a/Source/Core/Core/HW/GCMemcard/GCMemcard.cpp b/Source/Core/Core/HW/GCMemcard/GCMemcard.cpp index e2187c72b9..77f73a1224 100644 --- a/Source/Core/Core/HW/GCMemcard/GCMemcard.cpp +++ b/Source/Core/Core/HW/GCMemcard/GCMemcard.cpp @@ -1420,6 +1420,16 @@ s32 GCMemcard::PSO_MakeSaveGameValid(const Header& cardheader, const DEntry& dir return 1; } +GCMBlock::GCMBlock() +{ + Erase(); +} + +void GCMBlock::Erase() +{ + memset(m_block.data(), 0xFF, m_block.size()); +} + Header::Header(int slot, u16 size_mbits, bool shift_jis) { // Nintendo format algorithm. diff --git a/Source/Core/Core/HW/GCMemcard/GCMemcard.h b/Source/Core/Core/HW/GCMemcard/GCMemcard.h index 30ef094ddf..bc32974e3e 100644 --- a/Source/Core/Core/HW/GCMemcard/GCMemcard.h +++ b/Source/Core/Core/HW/GCMemcard/GCMemcard.h @@ -92,8 +92,8 @@ protected: struct GCMBlock { - GCMBlock() { Erase(); } - void Erase() { memset(m_block.data(), 0xFF, m_block.size()); } + GCMBlock(); + void Erase(); std::array m_block; }; From 6e04e4dd6afdfc0f3218377ed7a48763bf51f0fb Mon Sep 17 00:00:00 2001 From: "Admiral H. Curtiss" Date: Sun, 21 Apr 2019 17:40:35 +0200 Subject: [PATCH 05/12] GCMemcard: DEntry: Move code out of header. --- Source/Core/Core/HW/GCMemcard/GCMemcard.cpp | 14 ++++++++++++++ Source/Core/Core/HW/GCMemcard/GCMemcard.h | 13 ++++--------- 2 files changed, 18 insertions(+), 9 deletions(-) diff --git a/Source/Core/Core/HW/GCMemcard/GCMemcard.cpp b/Source/Core/Core/HW/GCMemcard/GCMemcard.cpp index 77f73a1224..c5a0a8c465 100644 --- a/Source/Core/Core/HW/GCMemcard/GCMemcard.cpp +++ b/Source/Core/Core/HW/GCMemcard/GCMemcard.cpp @@ -1470,6 +1470,20 @@ void Header::CalculateSerial(u32* serial1, u32* serial2) const *serial2 = serial[1] ^ serial[3] ^ serial[5] ^ serial[7]; } +DEntry::DEntry() +{ + memset(this, 0xFF, DENTRY_SIZE); +} + +std::string DEntry::GCI_FileName() const +{ + std::string filename = + std::string(reinterpret_cast(m_makercode.data()), m_makercode.size()) + '-' + + std::string(reinterpret_cast(m_gamecode.data()), m_gamecode.size()) + '-' + + reinterpret_cast(m_filename.data()) + ".gci"; + return Common::EscapeFileName(filename); +} + Directory::Directory() { memset(this, 0xFF, BLOCK_SIZE); diff --git a/Source/Core/Core/HW/GCMemcard/GCMemcard.h b/Source/Core/Core/HW/GCMemcard/GCMemcard.h index bc32974e3e..b3ecd41798 100644 --- a/Source/Core/Core/HW/GCMemcard/GCMemcard.h +++ b/Source/Core/Core/HW/GCMemcard/GCMemcard.h @@ -154,15 +154,10 @@ static_assert(sizeof(Header) == BLOCK_SIZE); struct DEntry { - DEntry() { memset(this, 0xFF, DENTRY_SIZE); } - std::string GCI_FileName() const - { - std::string filename = - std::string(reinterpret_cast(m_makercode.data()), m_makercode.size()) + '-' + - std::string(reinterpret_cast(m_gamecode.data()), m_gamecode.size()) + '-' + - reinterpret_cast(m_filename.data()) + ".gci"; - return Common::EscapeFileName(filename); - } + DEntry(); + + // TODO: This probably shouldn't be here at all? + std::string GCI_FileName() const; static constexpr std::array UNINITIALIZED_GAMECODE{{0xFF, 0xFF, 0xFF, 0xFF}}; From fcd75841ca2b20323aa1bbdf18bacd40d9dec488 Mon Sep 17 00:00:00 2001 From: "Admiral H. Curtiss" Date: Sun, 5 May 2019 00:59:38 +0200 Subject: [PATCH 06/12] GCMemcard: Rewrite Header::CalculateSerial() without undefined behavior. --- Source/Core/Core/HW/GCMemcard/GCMemcard.cpp | 23 ++++++++++++--------- Source/Core/Core/HW/GCMemcard/GCMemcard.h | 2 +- 2 files changed, 14 insertions(+), 11 deletions(-) diff --git a/Source/Core/Core/HW/GCMemcard/GCMemcard.cpp b/Source/Core/Core/HW/GCMemcard/GCMemcard.cpp index c5a0a8c465..c0dccde285 100644 --- a/Source/Core/Core/HW/GCMemcard/GCMemcard.cpp +++ b/Source/Core/Core/HW/GCMemcard/GCMemcard.cpp @@ -1306,7 +1306,6 @@ s32 GCMemcard::FZEROGX_MakeSaveGameValid(const Header& cardheader, const DEntry& std::vector& FileBuffer) { u32 i, j; - u32 serial1, serial2; u16 chksum = 0xFFFF; // check for F-Zero GX system file @@ -1318,7 +1317,7 @@ s32 GCMemcard::FZEROGX_MakeSaveGameValid(const Header& cardheader, const DEntry& return 0; // get encrypted destination memory card serial numbers - cardheader.CalculateSerial(&serial1, &serial2); + const auto [serial1, serial2] = cardheader.CalculateSerial(); // set new serial numbers *(u16*)&FileBuffer[1].m_block[0x0066] = BE16(BE32(serial1) >> 16); @@ -1364,7 +1363,6 @@ s32 GCMemcard::PSO_MakeSaveGameValid(const Header& cardheader, const DEntry& dir u32 i, j; u32 chksum; u32 crc32LUT[256]; - u32 serial1, serial2; u32 pso3offset = 0x00; // check for PSO1&2 system file @@ -1384,7 +1382,7 @@ s32 GCMemcard::PSO_MakeSaveGameValid(const Header& cardheader, const DEntry& dir } // get encrypted destination memory card serial numbers - cardheader.CalculateSerial(&serial1, &serial2); + const auto [serial1, serial2] = cardheader.CalculateSerial(); // set new serial numbers *(u32*)&FileBuffer[1].m_block[0x0158] = serial1; @@ -1457,17 +1455,22 @@ Header::Header(int slot, u16 size_mbits, bool shift_jis) calc_checksumsBE((u16*)this, 0xFE, &m_checksum, &m_checksum_inv); } -void Header::CalculateSerial(u32* serial1, u32* serial2) const +std::pair Header::CalculateSerial() const { - u32 serial[8]; + static_assert(std::is_trivially_copyable
()); - for (int i = 0; i < 8; i++) + std::array raw; + memcpy(raw.data(), this, raw.size()); + + u32 serial1 = 0; + u32 serial2 = 0; + for (size_t i = 0; i < raw.size(); i += 8) { - memcpy(&serial[i], (u8*)this + (i * 4), 4); + serial1 ^= Common::BitCastPtr(&raw[i + 0]); + serial2 ^= Common::BitCastPtr(&raw[i + 4]); } - *serial1 = serial[0] ^ serial[2] ^ serial[4] ^ serial[6]; - *serial2 = serial[1] ^ serial[3] ^ serial[5] ^ serial[7]; + return std::make_pair(serial1, serial2); } DEntry::DEntry() diff --git a/Source/Core/Core/HW/GCMemcard/GCMemcard.h b/Source/Core/Core/HW/GCMemcard/GCMemcard.h index b3ecd41798..7dc6ff2eec 100644 --- a/Source/Core/Core/HW/GCMemcard/GCMemcard.h +++ b/Source/Core/Core/HW/GCMemcard/GCMemcard.h @@ -148,7 +148,7 @@ struct Header explicit Header(int slot = 0, u16 size_mbits = MemCard2043Mb, bool shift_jis = false); // Calculates the card serial numbers used for encrypting some save files. - void CalculateSerial(u32* serial1, u32* serial2) const; + std::pair CalculateSerial() const; }; static_assert(sizeof(Header) == BLOCK_SIZE); From 88a0773309d658f64a2f958bb2e8e7ade1eb9798 Mon Sep 17 00:00:00 2001 From: "Admiral H. Curtiss" Date: Sun, 21 Apr 2019 19:21:23 +0200 Subject: [PATCH 07/12] GCMemcard: Rewrite checksum calculation without undefined behavior. --- Source/Core/Core/HW/GCMemcard/GCMemcard.cpp | 130 +++++++++++++------- Source/Core/Core/HW/GCMemcard/GCMemcard.h | 10 +- 2 files changed, 90 insertions(+), 50 deletions(-) diff --git a/Source/Core/Core/HW/GCMemcard/GCMemcard.cpp b/Source/Core/Core/HW/GCMemcard/GCMemcard.cpp index c0dccde285..6751669783 100644 --- a/Source/Core/Core/HW/GCMemcard/GCMemcard.cpp +++ b/Source/Core/Core/HW/GCMemcard/GCMemcard.cpp @@ -5,6 +5,7 @@ #include "Core/HW/GCMemcard/GCMemcard.h" #include +#include #include #include #include @@ -270,54 +271,50 @@ bool GCMemcard::Save() return mcdFile.Close(); } -void calc_checksumsBE(const u16* buf, u32 length, u16* csum, u16* inv_csum) +std::pair CalculateMemcardChecksums(const u8* data, size_t size) { - *csum = *inv_csum = 0; + assert(size % 2 == 0); + u16 csum = 0; + u16 inv_csum = 0; - for (u32 i = 0; i < length; ++i) + for (size_t i = 0; i < size; i += 2) { - // weird warnings here - *csum += BE16(buf[i]); - *inv_csum += BE16((u16)(buf[i] ^ 0xffff)); - } - *csum = BE16(*csum); - *inv_csum = BE16(*inv_csum); - if (*csum == 0xffff) - { - *csum = 0; - } - if (*inv_csum == 0xffff) - { - *inv_csum = 0; + u16 d = Common::swap16(&data[i]); + csum += d; + inv_csum += static_cast(d ^ 0xffff); } + + csum = Common::swap16(csum); + inv_csum = Common::swap16(inv_csum); + + if (csum == 0xffff) + csum = 0; + if (inv_csum == 0xffff) + inv_csum = 0; + + return std::make_pair(csum, inv_csum); } u32 GCMemcard::TestChecksums() const { - u16 csum = 0, csum_inv = 0; + const auto [csum_hdr, cinv_hdr] = m_header_block.CalculateChecksums(); + const auto [csum_dir0, cinv_dir0] = m_directory_blocks[0].CalculateChecksums(); + const auto [csum_dir1, cinv_dir1] = m_directory_blocks[1].CalculateChecksums(); + const auto [csum_bat0, cinv_bat0] = m_bat_blocks[0].CalculateChecksums(); + const auto [csum_bat1, cinv_bat1] = m_bat_blocks[1].CalculateChecksums(); u32 results = 0; - - calc_checksumsBE((u16*)&m_header_block, 0xFE, &csum, &csum_inv); - if ((m_header_block.m_checksum != csum) || (m_header_block.m_checksum_inv != csum_inv)) + if ((m_header_block.m_checksum != csum_hdr) || (m_header_block.m_checksum_inv != cinv_hdr)) results |= 1; - - calc_checksumsBE((u16*)&m_directory_blocks[0], 0xFFE, &csum, &csum_inv); - if ((m_directory_blocks[0].m_checksum != csum) || - (m_directory_blocks[0].m_checksum_inv != csum_inv)) + if ((m_directory_blocks[0].m_checksum != csum_dir0) || + (m_directory_blocks[0].m_checksum_inv != cinv_dir0)) results |= 2; - - calc_checksumsBE((u16*)&m_directory_blocks[1], 0xFFE, &csum, &csum_inv); - if ((m_directory_blocks[1].m_checksum != csum) || - (m_directory_blocks[1].m_checksum_inv != csum_inv)) + if ((m_directory_blocks[1].m_checksum != csum_dir1) || + (m_directory_blocks[1].m_checksum_inv != cinv_dir1)) results |= 4; - - calc_checksumsBE((u16*)(((u8*)&m_bat_blocks[0]) + 4), 0xFFE, &csum, &csum_inv); - if ((m_bat_blocks[0].m_checksum != csum) || (m_bat_blocks[0].m_checksum_inv != csum_inv)) + if ((m_bat_blocks[0].m_checksum != csum_bat0) || (m_bat_blocks[0].m_checksum_inv != cinv_bat0)) results |= 8; - - calc_checksumsBE((u16*)(((u8*)&m_bat_blocks[1]) + 4), 0xFFE, &csum, &csum_inv); - if ((m_bat_blocks[1].m_checksum != csum) || (m_bat_blocks[1].m_checksum_inv != csum_inv)) + if ((m_bat_blocks[1].m_checksum != csum_bat1) || (m_bat_blocks[1].m_checksum_inv != cinv_bat1)) results |= 16; return results; @@ -328,16 +325,11 @@ bool GCMemcard::FixChecksums() if (!m_valid) return false; - calc_checksumsBE((u16*)&m_header_block, 0xFE, &m_header_block.m_checksum, - &m_header_block.m_checksum_inv); - calc_checksumsBE((u16*)&m_directory_blocks[0], 0xFFE, &m_directory_blocks[0].m_checksum, - &m_directory_blocks[0].m_checksum_inv); - calc_checksumsBE((u16*)&m_directory_blocks[1], 0xFFE, &m_directory_blocks[1].m_checksum, - &m_directory_blocks[1].m_checksum_inv); - calc_checksumsBE((u16*)&m_bat_blocks[0] + 2, 0xFFE, &m_bat_blocks[0].m_checksum, - &m_bat_blocks[0].m_checksum_inv); - calc_checksumsBE((u16*)&m_bat_blocks[1] + 2, 0xFFE, &m_bat_blocks[1].m_checksum, - &m_bat_blocks[1].m_checksum_inv); + m_header_block.FixChecksums(); + m_directory_blocks[0].FixChecksums(); + m_directory_blocks[1].FixChecksums(); + m_bat_blocks[0].FixChecksums(); + m_bat_blocks[1].FixChecksums(); return true; } @@ -656,7 +648,7 @@ bool BlockAlloc::ClearBlocks(u16 starting_block, u16 block_count) void BlockAlloc::FixChecksums() { - calc_checksumsBE((u16*)&m_update_counter, 0xFFE, &m_checksum, &m_checksum_inv); + std::tie(m_checksum, m_checksum_inv) = CalculateChecksums(); } u16 BlockAlloc::AssignBlocksContiguous(u16 length) @@ -677,6 +669,19 @@ u16 BlockAlloc::AssignBlocksContiguous(u16 length) return starting; } +std::pair BlockAlloc::CalculateChecksums() const +{ + static_assert(std::is_trivially_copyable()); + + std::array raw; + memcpy(raw.data(), this, raw.size()); + + constexpr size_t checksum_area_start = offsetof(BlockAlloc, m_update_counter); + constexpr size_t checksum_area_end = sizeof(BlockAlloc); + constexpr size_t checksum_area_size = checksum_area_end - checksum_area_start; + return CalculateMemcardChecksums(&raw[checksum_area_start], checksum_area_size); +} + u32 GCMemcard::GetSaveData(u8 index, std::vector& Blocks) const { if (!m_valid) @@ -1452,7 +1457,7 @@ Header::Header(int slot, u16 size_mbits, bool shift_jis) memset(m_unknown_2.data(), 0, m_unknown_2.size()); // = _viReg[55]; static vu16* const _viReg = (u16*)0xCC002000; m_device_id = 0; - calc_checksumsBE((u16*)this, 0xFE, &m_checksum, &m_checksum_inv); + FixChecksums(); } std::pair Header::CalculateSerial() const @@ -1487,6 +1492,24 @@ std::string DEntry::GCI_FileName() const return Common::EscapeFileName(filename); } +void Header::FixChecksums() +{ + std::tie(m_checksum, m_checksum_inv) = CalculateChecksums(); +} + +std::pair Header::CalculateChecksums() const +{ + static_assert(std::is_trivially_copyable
()); + + std::array raw; + memcpy(raw.data(), this, raw.size()); + + constexpr size_t checksum_area_start = offsetof(Header, m_serial); + constexpr size_t checksum_area_end = offsetof(Header, m_checksum); + constexpr size_t checksum_area_size = checksum_area_end - checksum_area_start; + return CalculateMemcardChecksums(&raw[checksum_area_start], checksum_area_size); +} + Directory::Directory() { memset(this, 0xFF, BLOCK_SIZE); @@ -1507,5 +1530,18 @@ bool Directory::Replace(const DEntry& entry, size_t index) void Directory::FixChecksums() { - calc_checksumsBE((u16*)this, 0xFFE, &m_checksum, &m_checksum_inv); + std::tie(m_checksum, m_checksum_inv) = CalculateChecksums(); +} + +std::pair Directory::CalculateChecksums() const +{ + static_assert(std::is_trivially_copyable()); + + std::array raw; + memcpy(raw.data(), this, raw.size()); + + constexpr size_t checksum_area_start = offsetof(Directory, m_dir_entries); + constexpr size_t checksum_area_end = offsetof(Directory, m_checksum); + constexpr size_t checksum_area_size = checksum_area_end - checksum_area_start; + return CalculateMemcardChecksums(&raw[checksum_area_start], checksum_area_size); } diff --git a/Source/Core/Core/HW/GCMemcard/GCMemcard.h b/Source/Core/Core/HW/GCMemcard/GCMemcard.h index 7dc6ff2eec..3bcd579476 100644 --- a/Source/Core/Core/HW/GCMemcard/GCMemcard.h +++ b/Source/Core/Core/HW/GCMemcard/GCMemcard.h @@ -97,8 +97,6 @@ struct GCMBlock std::array m_block; }; -void calc_checksumsBE(const u16* buf, u32 length, u16* csum, u16* inv_csum); - #pragma pack(push, 1) struct Header { @@ -149,6 +147,9 @@ struct Header // Calculates the card serial numbers used for encrypting some save files. std::pair CalculateSerial() const; + + void FixChecksums(); + std::pair CalculateChecksums() const; }; static_assert(sizeof(Header) == BLOCK_SIZE); @@ -257,6 +258,7 @@ struct Directory bool Replace(const DEntry& entry, size_t index); void FixChecksums(); + std::pair CalculateChecksums() const; }; static_assert(sizeof(Directory) == BLOCK_SIZE); @@ -285,8 +287,10 @@ struct BlockAlloc u16 GetNextBlock(u16 block) const; u16 NextFreeBlock(u16 max_block, u16 starting_block = MC_FST_BLOCKS) const; bool ClearBlocks(u16 starting_block, u16 block_count); - void FixChecksums(); u16 AssignBlocksContiguous(u16 length); + + void FixChecksums(); + std::pair CalculateChecksums() const; }; static_assert(sizeof(BlockAlloc) == BLOCK_SIZE); #pragma pack(pop) From 2d3836441098770a92bdd094958b8c90bf6edb07 Mon Sep 17 00:00:00 2001 From: "Admiral H. Curtiss" Date: Sun, 21 Apr 2019 22:37:09 +0200 Subject: [PATCH 08/12] GCMemcard: Remove memsets that don't do anything in GCMemcard::Format(). --- Source/Core/Core/HW/GCMemcard/GCMemcard.cpp | 6 ------ 1 file changed, 6 deletions(-) diff --git a/Source/Core/Core/HW/GCMemcard/GCMemcard.cpp b/Source/Core/Core/HW/GCMemcard/GCMemcard.cpp index 6751669783..8b15387e84 100644 --- a/Source/Core/Core/HW/GCMemcard/GCMemcard.cpp +++ b/Source/Core/Core/HW/GCMemcard/GCMemcard.cpp @@ -1275,12 +1275,6 @@ bool GCMemcard::Format(u8* card_data, bool shift_jis, u16 SizeMb) bool GCMemcard::Format(bool shift_jis, u16 SizeMb) { - memset(&m_header_block, 0xFF, BLOCK_SIZE); - memset(&m_directory_blocks[0], 0xFF, BLOCK_SIZE); - memset(&m_directory_blocks[1], 0xFF, BLOCK_SIZE); - memset(&m_bat_blocks[0], 0, BLOCK_SIZE); - memset(&m_bat_blocks[1], 0, BLOCK_SIZE); - m_header_block = Header(SLOT_A, SizeMb, shift_jis); m_directory_blocks[0] = m_directory_blocks[1] = Directory(); m_bat_blocks[0] = m_bat_blocks[1] = BlockAlloc(SizeMb); From cbc5acb8cdac4fe32b1eaae5b866ef142557dee8 Mon Sep 17 00:00:00 2001 From: "Admiral H. Curtiss" Date: Sun, 5 May 2019 02:37:37 +0200 Subject: [PATCH 09/12] GCMemcard: Get rid of stray signed length in ImportGciInternal(). --- Source/Core/Core/HW/GCMemcard/GCMemcard.cpp | 6 +++--- Source/Core/Core/HW/GCMemcard/GCMemcard.h | 2 +- 2 files changed, 4 insertions(+), 4 deletions(-) diff --git a/Source/Core/Core/HW/GCMemcard/GCMemcard.cpp b/Source/Core/Core/HW/GCMemcard/GCMemcard.cpp index 8b15387e84..b3fe865ce1 100644 --- a/Source/Core/Core/HW/GCMemcard/GCMemcard.cpp +++ b/Source/Core/Core/HW/GCMemcard/GCMemcard.cpp @@ -882,9 +882,9 @@ u32 GCMemcard::ImportGciInternal(File::IOFile&& gci, const std::string& inputFil DEntry tempDEntry; gci.ReadBytes(&tempDEntry, DENTRY_SIZE); - const int fStart = (int)gci.Tell(); + const u64 fStart = gci.Tell(); gci.Seek(0, SEEK_END); - const int length = (int)gci.Tell() - fStart; + const u64 length = gci.Tell() - fStart; gci.Seek(offset + DENTRY_SIZE, SEEK_SET); Gcs_SavConvert(tempDEntry, offset, length); @@ -1022,7 +1022,7 @@ u32 GCMemcard::ExportGci(u8 index, const std::string& fileName, const std::strin return WRITEFAIL; } -void GCMemcard::Gcs_SavConvert(DEntry& tempDEntry, int saveType, int length) +void GCMemcard::Gcs_SavConvert(DEntry& tempDEntry, int saveType, u64 length) { switch (saveType) { diff --git a/Source/Core/Core/HW/GCMemcard/GCMemcard.h b/Source/Core/Core/HW/GCMemcard/GCMemcard.h index 3bcd579476..bd3c03ce55 100644 --- a/Source/Core/Core/HW/GCMemcard/GCMemcard.h +++ b/Source/Core/Core/HW/GCMemcard/GCMemcard.h @@ -420,7 +420,7 @@ public: // GCI files are untouched, SAV files are byteswapped // GCS files have the block count set, default is 1 (For export as GCS) - static void Gcs_SavConvert(DEntry& tempDEntry, int saveType, int length = BLOCK_SIZE); + static void Gcs_SavConvert(DEntry& tempDEntry, int saveType, u64 length = BLOCK_SIZE); // reads the banner image bool ReadBannerRGBA8(u8 index, u32* buffer) const; From d09303683cedb455c4a32acfe0a381e2eb48e572 Mon Sep 17 00:00:00 2001 From: "Admiral H. Curtiss" Date: Sun, 5 May 2019 02:55:03 +0200 Subject: [PATCH 10/12] GCMemcard: Convert a few enums into constexprs. --- Source/Core/Core/HW/GCMemcard/GCMemcard.h | 46 +++++++++++++++-------- 1 file changed, 31 insertions(+), 15 deletions(-) diff --git a/Source/Core/Core/HW/GCMemcard/GCMemcard.h b/Source/Core/Core/HW/GCMemcard/GCMemcard.h index bd3c03ce55..9365b66bba 100644 --- a/Source/Core/Core/HW/GCMemcard/GCMemcard.h +++ b/Source/Core/Core/HW/GCMemcard/GCMemcard.h @@ -40,7 +40,6 @@ enum LENGTHFAIL, INVALIDFILESIZE, TITLEPRESENT, - DIRLEN = 0x7F, SAV = 0x80, SAVFAIL, GCS = 0x110, @@ -49,25 +48,42 @@ enum WRITEFAIL, DELETE_FAIL, - MC_FST_BLOCKS = 0x05, - MBIT_TO_BLOCKS = 0x10, - DENTRY_STRLEN = 0x20, - DENTRY_SIZE = 0x40, - BLOCK_SIZE = 0x2000, - BAT_SIZE = 0xFFB, - - MemCard59Mb = 0x04, - MemCard123Mb = 0x08, - MemCard251Mb = 0x10, - Memcard507Mb = 0x20, - MemCard1019Mb = 0x40, - MemCard2043Mb = 0x80, - CI8SHARED = 1, RGB5A3, CI8, }; +// size of a single memory card block in bytes +constexpr u32 BLOCK_SIZE = 0x2000; + +// the amount of memory card blocks in a megabit of data +constexpr u32 MBIT_TO_BLOCKS = (1024 * 1024) / (BLOCK_SIZE * 8); + +// number of metadata and filesystem blocks before the actual user data blocks +constexpr u32 MC_FST_BLOCKS = 0x05; + +// maximum number of saves that can be stored on a single memory card +constexpr u8 DIRLEN = 0x7F; + +// maximum size of memory card file comment in bytes +constexpr u32 DENTRY_STRLEN = 0x20; + +// size of a single entry in the Directory in bytes +constexpr u32 DENTRY_SIZE = 0x40; + +// number of block entries in the BAT; one entry uses 2 bytes +constexpr u16 BAT_SIZE = 0xFFB; + +// possible sizes of memory cards in megabits +// TODO: Do memory card sizes have to be power of two? +// TODO: Are these all of them? A 4091 block card should work in theory at least. +constexpr u16 MemCard59Mb = 0x04; +constexpr u16 MemCard123Mb = 0x08; +constexpr u16 MemCard251Mb = 0x10; +constexpr u16 Memcard507Mb = 0x20; +constexpr u16 MemCard1019Mb = 0x40; +constexpr u16 MemCard2043Mb = 0x80; + class MemoryCardBase { public: From 018572018e2e898e388a0347a613d684b7925edc Mon Sep 17 00:00:00 2001 From: "Admiral H. Curtiss" Date: Sun, 5 May 2019 15:56:30 +0200 Subject: [PATCH 11/12] GCMemcard: Dismantle the global return value enum into a few function specific enum classes. --- Source/Core/Core/HW/GCMemcard/GCMemcard.cpp | 106 +++++++++++--------- Source/Core/Core/HW/GCMemcard/GCMemcard.h | 71 +++++++++---- Source/Core/DolphinQt/GCMemcardManager.cpp | 10 +- 3 files changed, 112 insertions(+), 75 deletions(-) diff --git a/Source/Core/Core/HW/GCMemcard/GCMemcard.cpp b/Source/Core/Core/HW/GCMemcard/GCMemcard.cpp index b3fe865ce1..10f18ee80e 100644 --- a/Source/Core/Core/HW/GCMemcard/GCMemcard.cpp +++ b/Source/Core/Core/HW/GCMemcard/GCMemcard.cpp @@ -682,10 +682,10 @@ std::pair BlockAlloc::CalculateChecksums() const return CalculateMemcardChecksums(&raw[checksum_area_start], checksum_area_size); } -u32 GCMemcard::GetSaveData(u8 index, std::vector& Blocks) const +GCMemcardGetSaveDataRetVal GCMemcard::GetSaveData(u8 index, std::vector& Blocks) const { if (!m_valid) - return NOMEMCARD; + return GCMemcardGetSaveDataRetVal::NOMEMCARD; u16 block = DEntry_FirstBlock(index); u16 BlockCount = DEntry_BlockCount(index); @@ -693,44 +693,45 @@ u32 GCMemcard::GetSaveData(u8 index, std::vector& Blocks) const if ((block == 0xFFFF) || (BlockCount == 0xFFFF)) { - return FAIL; + return GCMemcardGetSaveDataRetVal::FAIL; } u16 nextBlock = block; for (int i = 0; i < BlockCount; ++i) { if ((!nextBlock) || (nextBlock == 0xFFFF)) - return FAIL; + return GCMemcardGetSaveDataRetVal::FAIL; Blocks.push_back(m_data_blocks[nextBlock - MC_FST_BLOCKS]); nextBlock = GetActiveBat().GetNextBlock(nextBlock); } - return SUCCESS; + return GCMemcardGetSaveDataRetVal::SUCCESS; } // End DEntry functions -u32 GCMemcard::ImportFile(const DEntry& direntry, std::vector& saveBlocks) +GCMemcardImportFileRetVal GCMemcard::ImportFile(const DEntry& direntry, + std::vector& saveBlocks) { if (!m_valid) - return NOMEMCARD; + return GCMemcardImportFileRetVal::NOMEMCARD; if (GetNumFiles() >= DIRLEN) { - return OUTOFDIRENTRIES; + return GCMemcardImportFileRetVal::OUTOFDIRENTRIES; } if (GetActiveBat().m_free_blocks < direntry.m_block_count) { - return OUTOFBLOCKS; + return GCMemcardImportFileRetVal::OUTOFBLOCKS; } if (TitlePresent(direntry) != DIRLEN) { - return TITLEPRESENT; + return GCMemcardImportFileRetVal::TITLEPRESENT; } // find first free data block u16 firstBlock = GetActiveBat().NextFreeBlock(m_size_blocks, GetActiveBat().m_last_allocated_block); if (firstBlock == 0xFFFF) - return OUTOFBLOCKS; + return GCMemcardImportFileRetVal::OUTOFBLOCKS; Directory UpdatedDir = GetActiveDirectory(); // find first free dir entry @@ -775,22 +776,22 @@ u32 GCMemcard::ImportFile(const DEntry& direntry, std::vector& saveBlo FixChecksums(); - return SUCCESS; + return GCMemcardImportFileRetVal::SUCCESS; } -u32 GCMemcard::RemoveFile(u8 index) // index in the directory array +GCMemcardRemoveFileRetVal GCMemcard::RemoveFile(u8 index) // index in the directory array { if (!m_valid) - return NOMEMCARD; + return GCMemcardRemoveFileRetVal::NOMEMCARD; if (index >= DIRLEN) - return DELETE_FAIL; + return GCMemcardRemoveFileRetVal::DELETE_FAIL; u16 startingblock = GetActiveDirectory().m_dir_entries[index].m_first_block; u16 numberofblocks = GetActiveDirectory().m_dir_entries[index].m_block_count; BlockAlloc UpdatedBat = GetActiveBat(); if (!UpdatedBat.ClearBlocks(startingblock, numberofblocks)) - return DELETE_FAIL; + return GCMemcardRemoveFileRetVal::DELETE_FAIL; UpdatedBat.m_update_counter = UpdatedBat.m_update_counter + 1; UpdateBat(UpdatedBat); @@ -806,50 +807,52 @@ u32 GCMemcard::RemoveFile(u8 index) // index in the directory array FixChecksums(); - return SUCCESS; + return GCMemcardRemoveFileRetVal::SUCCESS; } -u32 GCMemcard::CopyFrom(const GCMemcard& source, u8 index) +GCMemcardImportFileRetVal GCMemcard::CopyFrom(const GCMemcard& source, u8 index) { if (!m_valid || !source.m_valid) - return NOMEMCARD; + return GCMemcardImportFileRetVal::NOMEMCARD; std::optional tempDEntry = source.GetDEntry(index); if (!tempDEntry) - return NOMEMCARD; + return GCMemcardImportFileRetVal::NOMEMCARD; u32 size = source.DEntry_BlockCount(index); if (size == 0xFFFF) - return INVALIDFILESIZE; + return GCMemcardImportFileRetVal::INVALIDFILESIZE; std::vector saveData; saveData.reserve(size); switch (source.GetSaveData(index, saveData)) { - case FAIL: - return FAIL; - case NOMEMCARD: - return NOMEMCARD; + case GCMemcardGetSaveDataRetVal::FAIL: + return GCMemcardImportFileRetVal::FAIL; + case GCMemcardGetSaveDataRetVal::NOMEMCARD: + return GCMemcardImportFileRetVal::NOMEMCARD; default: FixChecksums(); return ImportFile(*tempDEntry, saveData); } } -u32 GCMemcard::ImportGci(const std::string& inputFile, const std::string& outputFile) +GCMemcardImportFileRetVal GCMemcard::ImportGci(const std::string& inputFile, + const std::string& outputFile) { if (outputFile.empty() && !m_valid) - return OPENFAIL; + return GCMemcardImportFileRetVal::OPENFAIL; File::IOFile gci(inputFile, "rb"); if (!gci) - return OPENFAIL; + return GCMemcardImportFileRetVal::OPENFAIL; return ImportGciInternal(std::move(gci), inputFile, outputFile); } -u32 GCMemcard::ImportGciInternal(File::IOFile&& gci, const std::string& inputFile, - const std::string& outputFile) +GCMemcardImportFileRetVal GCMemcard::ImportGciInternal(File::IOFile&& gci, + const std::string& inputFile, + const std::string& outputFile) { unsigned int offset; std::string fileType; @@ -866,17 +869,17 @@ u32 GCMemcard::ImportGciInternal(File::IOFile&& gci, const std::string& inputFil if (!memcmp(tmp, "GCSAVE", 6)) // Header must be uppercase offset = GCS; else - return GCSFAIL; + return GCMemcardImportFileRetVal::GCSFAIL; } else if (!strcasecmp(fileType.c_str(), ".sav")) { if (!memcmp(tmp, "DATELGC_SAVE", 0xC)) // Header must be uppercase offset = SAV; else - return SAVFAIL; + return GCMemcardImportFileRetVal::SAVFAIL; } else - return OPENFAIL; + return GCMemcardImportFileRetVal::OPENFAIL; } gci.Seek(offset, SEEK_SET); @@ -890,9 +893,9 @@ u32 GCMemcard::ImportGciInternal(File::IOFile&& gci, const std::string& inputFil Gcs_SavConvert(tempDEntry, offset, length); if (length != tempDEntry.m_block_count * BLOCK_SIZE) - return LENGTHFAIL; + return GCMemcardImportFileRetVal::LENGTHFAIL; if (gci.Tell() != offset + DENTRY_SIZE) // Verify correct file position - return OPENFAIL; + return GCMemcardImportFileRetVal::OPENFAIL; u32 size = tempDEntry.m_block_count; std::vector saveData; @@ -904,14 +907,14 @@ u32 GCMemcard::ImportGciInternal(File::IOFile&& gci, const std::string& inputFil gci.ReadBytes(b.m_block.data(), b.m_block.size()); saveData.push_back(b); } - u32 ret; + GCMemcardImportFileRetVal ret; if (!outputFile.empty()) { File::IOFile gci2(outputFile, "wb"); bool completeWrite = true; if (!gci2) { - return OPENFAIL; + return GCMemcardImportFileRetVal::OPENFAIL; } gci2.Seek(0, SEEK_SET); @@ -926,10 +929,12 @@ u32 GCMemcard::ImportGciInternal(File::IOFile&& gci, const std::string& inputFil completeWrite = false; } + // TODO: This is interpreted as failure by the calling code if it only checks for SUCCESS. + // What is the logic here? if (completeWrite) - ret = GCS; + ret = GCMemcardImportFileRetVal::GCS; else - ret = WRITEFAIL; + ret = GCMemcardImportFileRetVal::WRITEFAIL; } else ret = ImportFile(tempDEntry, saveData); @@ -937,7 +942,8 @@ u32 GCMemcard::ImportGciInternal(File::IOFile&& gci, const std::string& inputFil return ret; } -u32 GCMemcard::ExportGci(u8 index, const std::string& fileName, const std::string& directory) const +GCMemcardExportFileRetVal GCMemcard::ExportGci(u8 index, const std::string& fileName, + const std::string& directory) const { File::IOFile gci; int offset = GCI; @@ -947,7 +953,7 @@ u32 GCMemcard::ExportGci(u8 index, const std::string& fileName, const std::strin std::string gciFilename; // GCI_FileName should only fail if the gamecode is 0xFFFFFFFF if (!GCI_FileName(index, gciFilename)) - return SUCCESS; + return GCMemcardExportFileRetVal::SUCCESS; gci.Open(directory + DIR_SEP + gciFilename, "wb"); } else @@ -966,7 +972,7 @@ u32 GCMemcard::ExportGci(u8 index, const std::string& fileName, const std::strin } if (!gci) - return OPENFAIL; + return GCMemcardExportFileRetVal::OPENFAIL; gci.Seek(0, SEEK_SET); @@ -989,7 +995,7 @@ u32 GCMemcard::ExportGci(u8 index, const std::string& fileName, const std::strin std::optional tempDEntry = GetDEntry(index); if (!tempDEntry) - return NOMEMCARD; + return GCMemcardExportFileRetVal::NOMEMCARD; Gcs_SavConvert(*tempDEntry, offset); gci.WriteBytes(&tempDEntry.value(), DENTRY_SIZE); @@ -997,7 +1003,7 @@ u32 GCMemcard::ExportGci(u8 index, const std::string& fileName, const std::strin u32 size = DEntry_BlockCount(index); if (size == 0xFFFF) { - return FAIL; + return GCMemcardExportFileRetVal::FAIL; } std::vector saveData; @@ -1005,10 +1011,10 @@ u32 GCMemcard::ExportGci(u8 index, const std::string& fileName, const std::strin switch (GetSaveData(index, saveData)) { - case FAIL: - return FAIL; - case NOMEMCARD: - return NOMEMCARD; + case GCMemcardGetSaveDataRetVal::FAIL: + return GCMemcardExportFileRetVal::FAIL; + case GCMemcardGetSaveDataRetVal::NOMEMCARD: + return GCMemcardExportFileRetVal::NOMEMCARD; } gci.Seek(DENTRY_SIZE + offset, SEEK_SET); for (unsigned int i = 0; i < size; ++i) @@ -1017,9 +1023,9 @@ u32 GCMemcard::ExportGci(u8 index, const std::string& fileName, const std::strin } if (gci.IsGood()) - return SUCCESS; + return GCMemcardExportFileRetVal::SUCCESS; else - return WRITEFAIL; + return GCMemcardExportFileRetVal::WRITEFAIL; } void GCMemcard::Gcs_SavConvert(DEntry& tempDEntry, int saveType, u64 length) diff --git a/Source/Core/Core/HW/GCMemcard/GCMemcard.h b/Source/Core/Core/HW/GCMemcard/GCMemcard.h index 9365b66bba..dc226b68c7 100644 --- a/Source/Core/Core/HW/GCMemcard/GCMemcard.h +++ b/Source/Core/Core/HW/GCMemcard/GCMemcard.h @@ -32,27 +32,55 @@ enum SLOT_A = 0, SLOT_B = 1, GCI = 0, - SUCCESS, - NOMEMCARD, - OPENFAIL, - OUTOFBLOCKS, - OUTOFDIRENTRIES, - LENGTHFAIL, - INVALIDFILESIZE, - TITLEPRESENT, SAV = 0x80, - SAVFAIL, GCS = 0x110, - GCSFAIL, - FAIL, - WRITEFAIL, - DELETE_FAIL, CI8SHARED = 1, RGB5A3, CI8, }; +enum class GCMemcardGetSaveDataRetVal +{ + SUCCESS, + FAIL, + NOMEMCARD, +}; + +enum class GCMemcardImportFileRetVal +{ + SUCCESS, + FAIL, + NOMEMCARD, + OUTOFDIRENTRIES, + OUTOFBLOCKS, + TITLEPRESENT, + INVALIDFILESIZE, + GCSFAIL, + SAVFAIL, + OPENFAIL, + LENGTHFAIL, + WRITEFAIL, + GCS, +}; + +enum class GCMemcardExportFileRetVal +{ + SUCCESS, + FAIL, + NOMEMCARD, + OPENFAIL, + WRITEFAIL, + UNUSED, +}; + +enum class GCMemcardRemoveFileRetVal +{ + SUCCESS, + NOMEMCARD, + DELETE_FAIL, +}; + // size of a single memory card block in bytes constexpr u32 BLOCK_SIZE = 0x2000; @@ -352,8 +380,8 @@ private: int m_active_directory; int m_active_bat; - u32 ImportGciInternal(File::IOFile&& gci, const std::string& inputFile, - const std::string& outputFile); + GCMemcardImportFileRetVal ImportGciInternal(File::IOFile&& gci, const std::string& inputFile, + const std::string& outputFile); void InitActiveDirBat(); const Directory& GetActiveDirectory() const; @@ -417,22 +445,23 @@ public: // Fetches a DEntry from the given file index. std::optional GetDEntry(u8 index) const; - u32 GetSaveData(u8 index, std::vector& saveBlocks) const; + GCMemcardGetSaveDataRetVal GetSaveData(u8 index, std::vector& saveBlocks) const; // adds the file to the directory and copies its contents - u32 ImportFile(const DEntry& direntry, std::vector& saveBlocks); + GCMemcardImportFileRetVal ImportFile(const DEntry& direntry, std::vector& saveBlocks); // delete a file from the directory - u32 RemoveFile(u8 index); + GCMemcardRemoveFileRetVal RemoveFile(u8 index); // reads a save from another memcard, and imports the data into this memcard - u32 CopyFrom(const GCMemcard& source, u8 index); + GCMemcardImportFileRetVal CopyFrom(const GCMemcard& source, u8 index); // reads a .gci/.gcs/.sav file and calls ImportFile or saves out a gci file - u32 ImportGci(const std::string& inputFile, const std::string& outputFile); + GCMemcardImportFileRetVal ImportGci(const std::string& inputFile, const std::string& outputFile); // writes a .gci file to disk containing index - u32 ExportGci(u8 index, const std::string& fileName, const std::string& directory) const; + GCMemcardExportFileRetVal ExportGci(u8 index, const std::string& fileName, + const std::string& directory) const; // GCI files are untouched, SAV files are byteswapped // GCS files have the block count set, default is 1 (For export as GCS) diff --git a/Source/Core/DolphinQt/GCMemcardManager.cpp b/Source/Core/DolphinQt/GCMemcardManager.cpp index 24ef4069e2..1bfdfd0c73 100644 --- a/Source/Core/DolphinQt/GCMemcardManager.cpp +++ b/Source/Core/DolphinQt/GCMemcardManager.cpp @@ -300,7 +300,9 @@ void GCMemcardManager::ExportFiles(bool prompt) QStringLiteral("/%1").arg(QString::fromStdString(gci_filename)); } - if (!memcard->ExportGci(file_index, path.toStdString(), "")) + // TODO: This is obviously intended to check for success instead. + const auto exportRetval = memcard->ExportGci(file_index, path.toStdString(), ""); + if (exportRetval == GCMemcardExportFileRetVal::UNUSED) { File::Delete(path.toStdString()); } @@ -330,7 +332,7 @@ void GCMemcardManager::ImportFile() const auto result = m_slot_memcard[m_active_slot]->ImportGci(path.toStdString(), ""); - if (result != SUCCESS) + if (result != GCMemcardImportFileRetVal::SUCCESS) { ModalMessageBox::critical(this, tr("Import failed"), tr("Failed to import \"%1\".").arg(path)); return; @@ -356,7 +358,7 @@ void GCMemcardManager::CopyFiles() const auto result = m_slot_memcard[!m_active_slot]->CopyFrom(*memcard, file_index); - if (result != SUCCESS) + if (result != GCMemcardImportFileRetVal::SUCCESS) { ModalMessageBox::warning(this, tr("Copy failed"), tr("Failed to copy file")); } @@ -400,7 +402,7 @@ void GCMemcardManager::DeleteFiles() for (int file_index : file_indices) { - if (memcard->RemoveFile(file_index) != SUCCESS) + if (memcard->RemoveFile(file_index) != GCMemcardRemoveFileRetVal::SUCCESS) { ModalMessageBox::warning(this, tr("Remove failed"), tr("Failed to remove file")); } From e390fd0f4e853ceb7beb23eb44a79b42486f66dd Mon Sep 17 00:00:00 2001 From: "Admiral H. Curtiss" Date: Sun, 5 May 2019 16:14:55 +0200 Subject: [PATCH 12/12] GCMemcard: Remove unused ability of ImportGci() to write a GCI file to disk. --- Source/Core/Core/HW/GCMemcard/GCMemcard.cpp | 44 +++------------------ Source/Core/Core/HW/GCMemcard/GCMemcard.h | 9 ++--- Source/Core/DolphinQt/GCMemcardManager.cpp | 2 +- 3 files changed, 9 insertions(+), 46 deletions(-) diff --git a/Source/Core/Core/HW/GCMemcard/GCMemcard.cpp b/Source/Core/Core/HW/GCMemcard/GCMemcard.cpp index 10f18ee80e..e121b887b0 100644 --- a/Source/Core/Core/HW/GCMemcard/GCMemcard.cpp +++ b/Source/Core/Core/HW/GCMemcard/GCMemcard.cpp @@ -837,22 +837,20 @@ GCMemcardImportFileRetVal GCMemcard::CopyFrom(const GCMemcard& source, u8 index) } } -GCMemcardImportFileRetVal GCMemcard::ImportGci(const std::string& inputFile, - const std::string& outputFile) +GCMemcardImportFileRetVal GCMemcard::ImportGci(const std::string& inputFile) { - if (outputFile.empty() && !m_valid) + if (!m_valid) return GCMemcardImportFileRetVal::OPENFAIL; File::IOFile gci(inputFile, "rb"); if (!gci) return GCMemcardImportFileRetVal::OPENFAIL; - return ImportGciInternal(std::move(gci), inputFile, outputFile); + return ImportGciInternal(std::move(gci), inputFile); } GCMemcardImportFileRetVal GCMemcard::ImportGciInternal(File::IOFile&& gci, - const std::string& inputFile, - const std::string& outputFile) + const std::string& inputFile) { unsigned int offset; std::string fileType; @@ -907,39 +905,7 @@ GCMemcardImportFileRetVal GCMemcard::ImportGciInternal(File::IOFile&& gci, gci.ReadBytes(b.m_block.data(), b.m_block.size()); saveData.push_back(b); } - GCMemcardImportFileRetVal ret; - if (!outputFile.empty()) - { - File::IOFile gci2(outputFile, "wb"); - bool completeWrite = true; - if (!gci2) - { - return GCMemcardImportFileRetVal::OPENFAIL; - } - gci2.Seek(0, SEEK_SET); - - if (!gci2.WriteBytes(&tempDEntry, DENTRY_SIZE)) - completeWrite = false; - int fileBlocks = tempDEntry.m_block_count; - gci2.Seek(DENTRY_SIZE, SEEK_SET); - - for (int i = 0; i < fileBlocks; ++i) - { - if (!gci2.WriteBytes(saveData[i].m_block.data(), saveData[i].m_block.size())) - completeWrite = false; - } - - // TODO: This is interpreted as failure by the calling code if it only checks for SUCCESS. - // What is the logic here? - if (completeWrite) - ret = GCMemcardImportFileRetVal::GCS; - else - ret = GCMemcardImportFileRetVal::WRITEFAIL; - } - else - ret = ImportFile(tempDEntry, saveData); - - return ret; + return ImportFile(tempDEntry, saveData); } GCMemcardExportFileRetVal GCMemcard::ExportGci(u8 index, const std::string& fileName, diff --git a/Source/Core/Core/HW/GCMemcard/GCMemcard.h b/Source/Core/Core/HW/GCMemcard/GCMemcard.h index dc226b68c7..75017fe28b 100644 --- a/Source/Core/Core/HW/GCMemcard/GCMemcard.h +++ b/Source/Core/Core/HW/GCMemcard/GCMemcard.h @@ -60,8 +60,6 @@ enum class GCMemcardImportFileRetVal SAVFAIL, OPENFAIL, LENGTHFAIL, - WRITEFAIL, - GCS, }; enum class GCMemcardExportFileRetVal @@ -380,8 +378,7 @@ private: int m_active_directory; int m_active_bat; - GCMemcardImportFileRetVal ImportGciInternal(File::IOFile&& gci, const std::string& inputFile, - const std::string& outputFile); + GCMemcardImportFileRetVal ImportGciInternal(File::IOFile&& gci, const std::string& inputFile); void InitActiveDirBat(); const Directory& GetActiveDirectory() const; @@ -456,8 +453,8 @@ public: // reads a save from another memcard, and imports the data into this memcard GCMemcardImportFileRetVal CopyFrom(const GCMemcard& source, u8 index); - // reads a .gci/.gcs/.sav file and calls ImportFile or saves out a gci file - GCMemcardImportFileRetVal ImportGci(const std::string& inputFile, const std::string& outputFile); + // reads a .gci/.gcs/.sav file and calls ImportFile + GCMemcardImportFileRetVal ImportGci(const std::string& inputFile); // writes a .gci file to disk containing index GCMemcardExportFileRetVal ExportGci(u8 index, const std::string& fileName, diff --git a/Source/Core/DolphinQt/GCMemcardManager.cpp b/Source/Core/DolphinQt/GCMemcardManager.cpp index 1bfdfd0c73..d763d01ebb 100644 --- a/Source/Core/DolphinQt/GCMemcardManager.cpp +++ b/Source/Core/DolphinQt/GCMemcardManager.cpp @@ -330,7 +330,7 @@ void GCMemcardManager::ImportFile() if (path.isEmpty()) return; - const auto result = m_slot_memcard[m_active_slot]->ImportGci(path.toStdString(), ""); + const auto result = m_slot_memcard[m_active_slot]->ImportGci(path.toStdString()); if (result != GCMemcardImportFileRetVal::SUCCESS) {