Merge pull request #3324 from lioncash/alloc

DiscIO: Get rid of a few explicit deletes

Source/Core/DiscIO/Blob.cpp

@@ -4,6 +4,7 @@
 
 #include <cstddef>
 #include <cstring>
+#include <limits>
 #include <memory>
 #include <string>
 
@@ -26,36 +27,27 @@ namespace DiscIO
 
 void SectorReader::SetSectorSize(int blocksize)
 {
-	for (int i = 0; i < CACHE_SIZE; i++)
-	{
-		m_cache[i] = new u8[blocksize];
-		m_cache_tags[i] = (u64)(s64) - 1;
-	}
+	for (auto& cache_entry : m_cache)
+		cache_entry.resize(blocksize);
+
+	m_cache_tags.fill(std::numeric_limits<u64>::max());
 	m_blocksize = blocksize;
 }
 
 SectorReader::~SectorReader()
 {
-	for (u8*& block : m_cache)
-	{
-		delete [] block;
-	}
 }
 
-const u8 *SectorReader::GetBlockData(u64 block_num)
+const std::vector<u8>& SectorReader::GetBlockData(u64 block_num)
 {
 	// TODO : Expand usage of the cache to more than one block :P
 	if (m_cache_tags[0] == block_num)
-	{
 		return m_cache[0];
-	}
-	else
-	{
-		GetBlock(block_num, m_cache[0]);
+
+	GetBlock(block_num, m_cache[0].data());
 	m_cache_tags[0] = block_num;
 	return m_cache[0];
 }
-}
 
 bool SectorReader::Read(u64 offset, u64 size, u8* out_ptr)
 {
@@ -78,22 +70,20 @@ bool SectorReader::Read(u64 offset, u64 size, u8* out_ptr)
 			continue;
 		}
 
-		const u8* data = GetBlockData(block);
-		if (!data)
-			return false;
+		const std::vector<u8>& data = GetBlockData(block);
 
-		u32 toCopy = m_blocksize - positionInBlock;
-		if (toCopy >= remain)
+		u32 to_copy = m_blocksize - positionInBlock;
+		if (to_copy >= remain)
 		{
 			// Yay, we are done!
-			memcpy(out_ptr, data + positionInBlock, (size_t)remain);
+			std::copy(data.begin() + positionInBlock, data.begin() + positionInBlock + remain, out_ptr);
 			return true;
 		}
 		else
 		{
-			memcpy(out_ptr, data + positionInBlock, toCopy);
-			out_ptr += toCopy;
-			remain -= toCopy;
+			std::copy(data.begin() + positionInBlock, data.begin() + positionInBlock + to_copy, out_ptr);
+			out_ptr += to_copy;
+			remain -= to_copy;
 			positionInBlock = 0;
 			block++;
 		}
@@ -106,10 +96,10 @@ bool SectorReader::ReadMultipleAlignedBlocks(u64 block_num, u64 num_blocks, u8 *
 {
 	for (u64 i = 0; i < num_blocks; i++)
 	{
-		const u8 *data = GetBlockData(block_num + i);
-		if (!data)
-			return false;
-		memcpy(out_ptr + i * m_blocksize, data, m_blocksize);
+		const std::vector<u8>& data = GetBlockData(block_num + i);
+		const u64 offset = i * m_blocksize;
+
+		std::copy(data.begin(), data.end(), out_ptr + offset);
 	}
 
 	return true;

Source/Core/DiscIO/Blob.h

@@ -14,6 +14,7 @@
 // detect whether the file is a compressed blob, or just a big hunk of data, or a drive, and
 // automatically do the right thing.
 
+#include <array>
 #include <memory>
 #include <string>
 #include "Common/CommonTypes.h"
@@ -57,8 +58,6 @@ class SectorReader : public IBlobReader
 public:
 	virtual ~SectorReader();
 
-	// A pointer returned by GetBlockData is invalidated as soon as GetBlockData, Read, or ReadMultipleAlignedBlocks is called again.
-	const u8 *GetBlockData(u64 block_num);
 	bool Read(u64 offset, u64 size, u8 *out_ptr) override;
 	friend class DriveReader;
 
@@ -69,10 +68,13 @@ protected:
 	virtual bool ReadMultipleAlignedBlocks(u64 block_num, u64 num_blocks, u8 *out_ptr);
 
 private:
+	// A reference returned by GetBlockData is invalidated as soon as GetBlockData, Read, or ReadMultipleAlignedBlocks is called again.
+	const std::vector<u8>& GetBlockData(u64 block_num);
+
 	enum { CACHE_SIZE = 32 };
 	int m_blocksize;
-	u8* m_cache[CACHE_SIZE];
-	u64 m_cache_tags[CACHE_SIZE];
+	std::array<std::vector<u8>, CACHE_SIZE> m_cache;
+	std::array<u64, CACHE_SIZE> m_cache_tags;
 };
 
 // Factory function - examines the path to choose the right type of IBlobReader, and returns one.

Source/Core/DiscIO/CompressedBlob.cpp

@@ -39,10 +39,10 @@ CompressedBlobReader::CompressedBlobReader(const std::string& filename) : m_file
 	SetSectorSize(m_header.block_size);
 
 	// cache block pointers and hashes
-	m_block_pointers = new u64[m_header.num_blocks];
-	m_file.ReadArray(m_block_pointers, m_header.num_blocks);
-	m_hashes = new u32[m_header.num_blocks];
-	m_file.ReadArray(m_hashes, m_header.num_blocks);
+	m_block_pointers.resize(m_header.num_blocks);
+	m_file.ReadArray(m_block_pointers.data(), m_header.num_blocks);
+	m_hashes.resize(m_header.num_blocks);
+	m_file.ReadArray(m_hashes.data(), m_header.num_blocks);
 
 	m_data_offset = (sizeof(CompressedBlobHeader))
 	              + (sizeof(u64)) * m_header.num_blocks  // skip block pointers
@@ -50,9 +50,8 @@ CompressedBlobReader::CompressedBlobReader(const std::string& filename) : m_file
 
 	// A compressed block is never ever longer than a decompressed block, so just header.block_size should be fine.
 	// I still add some safety margin.
-	m_zlib_buffer_size = m_header.block_size + 64;
-	m_zlib_buffer = new u8[m_zlib_buffer_size];
-	memset(m_zlib_buffer, 0, m_zlib_buffer_size);
+	const u32 zlib_buffer_size = m_header.block_size + 64;
+	m_zlib_buffer.resize(zlib_buffer_size);
 }
 
 std::unique_ptr<CompressedBlobReader> CompressedBlobReader::Create(const std::string& filename)
@@ -65,9 +64,6 @@ std::unique_ptr<CompressedBlobReader> CompressedBlobReader::Create(const std::st
 
 CompressedBlobReader::~CompressedBlobReader()
 {
-	delete [] m_zlib_buffer;
-	delete [] m_block_pointers;
-	delete [] m_hashes;
 }
 
 // IMPORTANT: Calling this function invalidates all earlier pointers gotten from this function.
@@ -98,16 +94,13 @@ void CompressedBlobReader::GetBlock(u64 block_num, u8 *out_ptr)
 	}
 
 	// clear unused part of zlib buffer. maybe this can be deleted when it works fully.
-	memset(m_zlib_buffer + comp_block_size, 0, m_zlib_buffer_size - comp_block_size);
+	memset(&m_zlib_buffer[comp_block_size], 0, m_zlib_buffer.size() - comp_block_size);
 
 	m_file.Seek(offset, SEEK_SET);
-	m_file.ReadBytes(m_zlib_buffer, comp_block_size);
-
-	u8* source = m_zlib_buffer;
-	u8* dest = out_ptr;
+	m_file.ReadBytes(m_zlib_buffer.data(), comp_block_size);
 
 	// First, check hash.
-	u32 block_hash = HashAdler32(source, comp_block_size);
+	u32 block_hash = HashAdler32(m_zlib_buffer.data(), comp_block_size);
 	if (block_hash != m_hashes[block_num])
 		PanicAlertT("The disc image \"%s\" is corrupt.\n"
 		            "Hash of block %" PRIu64 " is %08x instead of %08x.",
@@ -116,19 +109,18 @@ void CompressedBlobReader::GetBlock(u64 block_num, u8 *out_ptr)
 
 	if (uncompressed)
 	{
-		memcpy(dest, source, comp_block_size);
+		std::copy(m_zlib_buffer.begin(), m_zlib_buffer.begin() + comp_block_size, out_ptr);
 	}
 	else
 	{
-		z_stream z;
-		memset(&z, 0, sizeof(z));
-		z.next_in  = source;
+		z_stream z = {};
+		z.next_in  = m_zlib_buffer.data();
 		z.avail_in = comp_block_size;
 		if (z.avail_in > m_header.block_size)
 		{
 			PanicAlert("We have a problem");
 		}
-		z.next_out  = dest;
+		z.next_out  = out_ptr;
 		z.avail_out = m_header.block_size;
 		inflateInit(&z);
 		int status = inflate(&z, Z_FULL_FLUSH);
@@ -201,10 +193,10 @@ bool CompressFileToBlob(const std::string& infile, const std::string& outfile, u
 	// round upwards!
 	header.num_blocks = (u32)((header.data_size + (block_size - 1)) / block_size);
 
-	u64* offsets = new u64[header.num_blocks];
-	u32* hashes = new u32[header.num_blocks];
-	u8* out_buf = new u8[block_size];
-	u8* in_buf = new u8[block_size];
+	std::vector<u64> offsets(header.num_blocks);
+	std::vector<u32> hashes(header.num_blocks);
+	std::vector<u8> out_buf(block_size);
+	std::vector<u8> in_buf(block_size);
 
 	// seek past the header (we will write it at the end)
 	f.Seek(sizeof(CompressedBlobHeader), SEEK_CUR);
@@ -240,16 +232,16 @@ bool CompressFileToBlob(const std::string& infile, const std::string& outfile, u
 
 		size_t read_bytes;
 		if (scrubbing)
-			read_bytes = DiscScrubber::GetNextBlock(inf, in_buf);
+			read_bytes = DiscScrubber::GetNextBlock(inf, in_buf.data());
 		else
-			inf.ReadArray(in_buf, header.block_size, &read_bytes);
+			inf.ReadArray(in_buf.data(), header.block_size, &read_bytes);
 		if (read_bytes < header.block_size)
-			std::fill(in_buf + read_bytes, in_buf + header.block_size, 0);
+			std::fill(in_buf.begin() + read_bytes, in_buf.begin() + header.block_size, 0);
 
 		int retval = deflateReset(&z);
-		z.next_in   = in_buf;
+		z.next_in   = in_buf.data();
 		z.avail_in  = header.block_size;
-		z.next_out  = out_buf;
+		z.next_out  = out_buf.data();
 		z.avail_out = block_size;
 
 		if (retval != Z_OK)
@@ -268,7 +260,7 @@ bool CompressFileToBlob(const std::string& infile, const std::string& outfile, u
 		{
 			//PanicAlert("%i %i Store %i", i*block_size, position, comp_size);
 			// let's store uncompressed
-			write_buf = in_buf;
+			write_buf = in_buf.data();
 			offsets[i] |= 0x8000000000000000ULL;
 			write_size = block_size;
 			num_stored++;
@@ -277,7 +269,7 @@ bool CompressFileToBlob(const std::string& infile, const std::string& outfile, u
 		{
 			// let's store compressed
 			//PanicAlert("Comp %i to %i", block_size, comp_size);
-			write_buf = out_buf;
+			write_buf = out_buf.data();
 			write_size = comp_size;
 			num_compressed++;
 		}
@@ -310,16 +302,11 @@ bool CompressFileToBlob(const std::string& infile, const std::string& outfile, u
 		// Okay, go back and fill in headers
 		f.Seek(0, SEEK_SET);
 		f.WriteArray(&header, 1);
-		f.WriteArray(offsets, header.num_blocks);
-		f.WriteArray(hashes, header.num_blocks);
+		f.WriteArray(offsets.data(), header.num_blocks);
+		f.WriteArray(hashes.data(), header.num_blocks);
 	}
 
 	// Cleanup
-	delete[] in_buf;
-	delete[] out_buf;
-	delete[] offsets;
-	delete[] hashes;
-
 	deflateEnd(&z);
 	DiscScrubber::Cleanup();
 

Source/Core/DiscIO/CompressedBlob.h

@@ -16,6 +16,7 @@
 
 #include <memory>
 #include <string>
+#include <vector>
 
 #include "Common/CommonTypes.h"
 #include "Common/FileUtil.h"
@@ -59,13 +60,12 @@ private:
 	CompressedBlobReader(const std::string& filename);
 
 	CompressedBlobHeader m_header;
-	u64* m_block_pointers;
-	u32* m_hashes;
+	std::vector<u64> m_block_pointers;
+	std::vector<u32> m_hashes;
 	int m_data_offset;
 	File::IOFile m_file;
 	u64 m_file_size;
-	u8* m_zlib_buffer;
-	int m_zlib_buffer_size;
+	std::vector<u8> m_zlib_buffer;
 	std::string m_file_name;
 };
 

Source/Core/DiscIO/DriveBlob.cpp

@@ -2,10 +2,12 @@
 // Licensed under GPLv2+
 // Refer to the license.txt file included.
 
+#include <algorithm>
 #include <cstdio>
 #include <cstring>
 #include <memory>
 #include <string>
+#include <vector>
 
 #include "Common/CommonTypes.h"
 #include "Common/FileUtil.h"
@@ -33,16 +35,14 @@ DriveReader::DriveReader(const std::string& drive)
 		// Do a test read to make sure everything is OK, since it seems you can get
 		// handles to empty drives.
 		DWORD not_used;
-		u8 *buffer = new u8[m_blocksize];
-		if (!ReadFile(m_disc_handle, buffer, m_blocksize, (LPDWORD)&not_used, nullptr))
+		std::vector<u8> buffer(m_blocksize);
+		if (!ReadFile(m_disc_handle, buffer.data(), m_blocksize, &not_used, nullptr))
 		{
-			delete [] buffer;
 			// OK, something is wrong.
 			CloseHandle(m_disc_handle);
 			m_disc_handle = INVALID_HANDLE_VALUE;
 			return;
 		}
-		delete [] buffer;
 
 	#ifdef _LOCKDRIVE // Do we want to lock the drive?
 		// Lock the compact disc in the CD-ROM drive to prevent accidental
@@ -97,32 +97,33 @@ std::unique_ptr<DriveReader> DriveReader::Create(const std::string& drive)
 
 void DriveReader::GetBlock(u64 block_num, u8* out_ptr)
 {
-	u8* const lpSector = new u8[m_blocksize];
+	std::vector<u8> sector(m_blocksize);
+
 #ifdef _WIN32
-	u32 NotUsed;
 	u64 offset = m_blocksize * block_num;
 	LONG off_low = (LONG)offset & 0xFFFFFFFF;
 	LONG off_high = (LONG)(offset >> 32);
+	DWORD not_used;
 	SetFilePointer(m_disc_handle, off_low, &off_high, FILE_BEGIN);
-	if (!ReadFile(m_disc_handle, lpSector, m_blocksize, (LPDWORD)&NotUsed, nullptr))
+	if (!ReadFile(m_disc_handle, sector.data(), m_blocksize, &not_used, nullptr))
 		PanicAlertT("Disc Read Error");
 #else
 	m_file.Seek(m_blocksize * block_num, SEEK_SET);
-	m_file.ReadBytes(lpSector, m_blocksize);
+	m_file.ReadBytes(sector.data(), m_blocksize);
 #endif
-	memcpy(out_ptr, lpSector, m_blocksize);
-	delete[] lpSector;
+
+	std::copy(sector.begin(), sector.end(), out_ptr);
 }
 
 bool DriveReader::ReadMultipleAlignedBlocks(u64 block_num, u64 num_blocks, u8* out_ptr)
 {
 #ifdef _WIN32
-	u32 NotUsed;
 	u64 offset = m_blocksize * block_num;
 	LONG off_low = (LONG)offset & 0xFFFFFFFF;
 	LONG off_high = (LONG)(offset >> 32);
+	DWORD not_used;
 	SetFilePointer(m_disc_handle, off_low, &off_high, FILE_BEGIN);
-	if (!ReadFile(m_disc_handle, out_ptr, (DWORD)(m_blocksize * num_blocks), (LPDWORD)&NotUsed, nullptr))
+	if (!ReadFile(m_disc_handle, out_ptr, (DWORD)(m_blocksize * num_blocks), &not_used, nullptr))
 	{
 		PanicAlertT("Disc Read Error");
 		return false;

Source/Core/DiscIO/DriveBlob.h

@@ -28,11 +28,10 @@ public:
 	u64 GetDataSize() const override { return m_size; }
 	u64 GetRawSize() const override { return m_size; }
 
-	bool ReadMultipleAlignedBlocks(u64 block_num, u64 num_blocks, u8 *out_ptr) override;
-
 private:
 	DriveReader(const std::string& drive);
 	void GetBlock(u64 block_num, u8 *out_ptr) override;
+	bool ReadMultipleAlignedBlocks(u64 block_num, u64 num_blocks, u8* out_ptr) override;
 
 #ifdef _WIN32
 	HANDLE m_disc_handle;

Source/Core/DiscIO/VolumeDirectory.cpp

@@ -29,7 +29,7 @@ CVolumeDirectory::CVolumeDirectory(const std::string& _rDirectory, bool _bIsWii,
 	: m_totalNameSize(0)
 	, m_dataStartAddress(-1)
 	, m_diskHeader(DISKHEADERINFO_ADDRESS)
-	, m_diskHeaderInfo(new SDiskHeaderInfo())
+	, m_diskHeaderInfo(std::make_unique<SDiskHeaderInfo>())
 	, m_fst_address(0)
 	, m_dol_address(0)
 {

Source/Core/DiscIO/VolumeWiiCrypted.cpp

@@ -30,14 +30,12 @@ namespace DiscIO
 CVolumeWiiCrypted::CVolumeWiiCrypted(std::unique_ptr<IBlobReader> reader, u64 _VolumeOffset,
 									 const unsigned char* _pVolumeKey)
 	: m_pReader(std::move(reader)),
-	m_AES_ctx(new mbedtls_aes_context),
-	m_pBuffer(nullptr),
+	m_AES_ctx(std::make_unique<mbedtls_aes_context>()),
 	m_VolumeOffset(_VolumeOffset),
 	m_dataOffset(0x20000),
 	m_LastDecryptedBlockOffset(-1)
 {
 	mbedtls_aes_setkey_dec(m_AES_ctx.get(), _pVolumeKey, 128);
-	m_pBuffer = new u8[s_block_total_size];
 }
 
 bool CVolumeWiiCrypted::ChangePartition(u64 offset)
@@ -53,8 +51,6 @@ bool CVolumeWiiCrypted::ChangePartition(u64 offset)
 
 CVolumeWiiCrypted::~CVolumeWiiCrypted()
 {
-	delete[] m_pBuffer;
-	m_pBuffer = nullptr;
 }
 
 bool CVolumeWiiCrypted::Read(u64 _ReadOffset, u64 _Length, u8* _pBuffer, bool decrypt) const
@@ -67,6 +63,7 @@ bool CVolumeWiiCrypted::Read(u64 _ReadOffset, u64 _Length, u8* _pBuffer, bool de
 
 	FileMon::FindFilename(_ReadOffset);
 
+	std::vector<u8> read_buffer(s_block_total_size);
 	while (_Length > 0)
 	{
 		// Calculate block offset
@@ -76,15 +73,15 @@ bool CVolumeWiiCrypted::Read(u64 _ReadOffset, u64 _Length, u8* _pBuffer, bool de
 		if (m_LastDecryptedBlockOffset != Block)
 		{
 			// Read the current block
-			if (!m_pReader->Read(m_VolumeOffset + m_dataOffset + Block * s_block_total_size, s_block_total_size, m_pBuffer))
+			if (!m_pReader->Read(m_VolumeOffset + m_dataOffset + Block * s_block_total_size, s_block_total_size, read_buffer.data()))
 				return false;
 
 			// Decrypt the block's data.
 			// 0x3D0 - 0x3DF in m_pBuffer will be overwritten,
 			// but that won't affect anything, because we won't
 			// use the content of m_pBuffer anymore after this
-			mbedtls_aes_crypt_cbc(m_AES_ctx.get(), MBEDTLS_AES_DECRYPT, s_block_data_size, m_pBuffer + 0x3D0,
-			              m_pBuffer + s_block_header_size, m_LastDecryptedBlock);
+			mbedtls_aes_crypt_cbc(m_AES_ctx.get(), MBEDTLS_AES_DECRYPT, s_block_data_size, &read_buffer[0x3D0],
+			              &read_buffer[s_block_header_size], m_LastDecryptedBlock);
 			m_LastDecryptedBlockOffset = Block;
 
 			// The only thing we currently use from the 0x000 - 0x3FF part

Source/Core/DiscIO/VolumeWiiCrypted.h

@@ -7,7 +7,6 @@
 #include <map>
 #include <memory>
 #include <string>
-#include <vector>
 #include <mbedtls/aes.h>
 
 #include "Common/CommonTypes.h"
@@ -46,8 +45,6 @@ public:
 	u64 GetSize() const override;
 	u64 GetRawSize() const override;
 
-
-
 private:
 	static const unsigned int s_block_header_size = 0x0400;
 	static const unsigned int s_block_data_size   = 0x7C00;
@@ -56,8 +53,6 @@ private:
 	std::unique_ptr<IBlobReader> m_pReader;
 	std::unique_ptr<mbedtls_aes_context> m_AES_ctx;
 
-	u8* m_pBuffer;
-
 	u64 m_VolumeOffset;
 	u64 m_dataOffset;
 

Source/Core/DiscIO/WbfsBlob.cpp

@@ -28,7 +28,7 @@ static inline u64 align(u64 value, u64 bounds)
 }
 
 WbfsFileReader::WbfsFileReader(const std::string& filename)
-	: m_total_files(0), m_size(0), m_wlba_table(nullptr), m_good(true)
+	: m_total_files(0), m_size(0), m_good(true)
 {
 	if (filename.length() < 4 || !OpenFiles(filename) || !ReadHeader())
 	{
@@ -37,19 +37,15 @@ WbfsFileReader::WbfsFileReader(const std::string& filename)
 	}
 
 	// Grab disc info (assume slot 0, checked in ReadHeader())
-	m_wlba_table = new u16[m_blocks_per_disc];
+	m_wlba_table.resize(m_blocks_per_disc);
 	m_files[0]->file.Seek(m_hd_sector_size + WII_DISC_HEADER_SIZE /*+ i * m_disc_info_size*/, SEEK_SET);
-	m_files[0]->file.ReadBytes(m_wlba_table, m_blocks_per_disc * sizeof(u16));
+	m_files[0]->file.ReadBytes(m_wlba_table.data(), m_blocks_per_disc * sizeof(u16));
 	for (size_t i = 0; i < m_blocks_per_disc; i++)
 		m_wlba_table[i] = Common::swap16(m_wlba_table[i]);
 }
 
 WbfsFileReader::~WbfsFileReader()
 {
-	for (file_entry* entry : m_files)
-		delete entry;
-
-	delete[] m_wlba_table;
 }
 
 u64 WbfsFileReader::GetDataSize() const
@@ -63,7 +59,7 @@ bool WbfsFileReader::OpenFiles(const std::string& filename)
 
 	while (true)
 	{
-		file_entry* new_entry = new file_entry;
+		auto new_entry = std::make_unique<file_entry>();
 
 		// Replace last character with index (e.g. wbfs = wbf1)
 		std::string path = filename;
@@ -74,8 +70,7 @@ bool WbfsFileReader::OpenFiles(const std::string& filename)
 
 		if (!new_entry->file.Open(path, "rb"))
 		{
-			delete new_entry;
-			return 0 != m_total_files;
+			return m_total_files != 0;
 		}
 
 		new_entry->base_address = m_size;
@@ -83,7 +78,7 @@ bool WbfsFileReader::OpenFiles(const std::string& filename)
 		m_size += new_entry->size;
 
 		m_total_files++;
-		m_files.push_back(new_entry);
+		m_files.emplace_back(std::move(new_entry));
 	}
 }
 

Source/Core/DiscIO/WbfsBlob.h

@@ -49,7 +49,7 @@ private:
 		u64 size;
 	};
 
-	std::vector<file_entry*> m_files;
+	std::vector<std::unique_ptr<file_entry>> m_files;
 
 	u32 m_total_files;
 	u64 m_size;
@@ -71,7 +71,7 @@ private:
 	} m_header;
 #pragma pack()
 
-	u16* m_wlba_table;
+	std::vector<u16> m_wlba_table;
 	u64 m_blocks_per_disc;
 
 	bool m_good;