Wrapped fopen/close/read/write functions inside a simple "IOFile" class. Reading, writing, and error checking became simpler in most cases. It should be near impossible to forget to close a file now that the destructor takes care of it. (I hope this fixes Issue 3635) I have tested the functionality of most things, but it is possible I broke something. :p

git-svn-id: https://dolphin-emu.googlecode.com/svn/trunk@7328 8ced0084-cf51-0410-be5f-012b33b47a6e

Source/Core/Common/Src/ChunkFile.h

@@ -27,8 +27,6 @@
 // - Zero backwards/forwards compatibility
 // - Serialization code for anything complex has to be manually written.
 
-#include <stdio.h>
-
 #include <map>
 #include <vector>
 #include <string>
@@ -189,63 +187,58 @@ public:
 			return false;
 				
 		// Check file size
-		u64 fileSize = File::GetSize(_rFilename);
-		u64 headerSize = sizeof(SChunkHeader);
-		if (fileSize < headerSize) {
+		const u64 fileSize = File::GetSize(_rFilename);
+		static const u64 headerSize = sizeof(SChunkHeader);
+		if (fileSize < headerSize)
+		{
 			ERROR_LOG(COMMON,"ChunkReader: File too small");
 			return false;
 		}
 
-		FILE* pFile = fopen(_rFilename.c_str(), "rb");
-		if (!pFile) { 
+		File::IOFile pFile(_rFilename, "rb");
+		if (!pFile)
+		{
 			ERROR_LOG(COMMON,"ChunkReader: Can't open file for reading");
 			return false;
 		}
 
 		// read the header
 		SChunkHeader header;
-		if (fread(&header, 1, headerSize, pFile) != headerSize) {
-			fclose(pFile);
+		if (!pFile.ReadArray(&header, 1))
+		{
 			ERROR_LOG(COMMON,"ChunkReader: Bad header size");
 			return false;
 		}
 		
 		// Check revision
-		if (header.Revision != _Revision) {
-			fclose(pFile);
+		if (header.Revision != _Revision)
+		{
 			ERROR_LOG(COMMON,"ChunkReader: Wrong file revision, got %d expected %d",
-					  header.Revision, _Revision);
+				header.Revision, _Revision);
 			return false;
 		}
 		
 		// get size
-		int sz = (int)(fileSize - headerSize);
-		if (header.ExpectedSize != sz) {
-			fclose(pFile);
+		const int sz = (int)(fileSize - headerSize);
+		if (header.ExpectedSize != sz)
+		{
 			ERROR_LOG(COMMON,"ChunkReader: Bad file size, got %d expected %d",
-					  sz, header.ExpectedSize);
+				sz, header.ExpectedSize);
 			return false;
 		}
 		
 		// read the state
 		u8* buffer = new u8[sz];
-		if ((int)fread(buffer, 1, sz, pFile) != sz) {
-			fclose(pFile);
+		if (!pFile.ReadBytes(buffer, sz))
+		{
 			ERROR_LOG(COMMON,"ChunkReader: Error reading file");
 			return false;
 		}
 
-		// done reading
-		if (fclose(pFile)) {
-			ERROR_LOG(COMMON,"ChunkReader: Error closing file! might be corrupted!");
-			// should never happen!
-			return false;
-		}
-
 		u8 *ptr = buffer;
 		PointerWrap p(&ptr, PointerWrap::MODE_READ);
 		_class.DoState(p);
-		delete [] buffer;
+		delete[] buffer;
 		
 		INFO_LOG(COMMON, "ChunkReader: Done loading %s" , _rFilename.c_str());
 		return true;
@@ -255,11 +248,10 @@ public:
 	template<class T>
 	static bool Save(const std::string& _rFilename, int _Revision, T& _class)
 	{
-		FILE* pFile;
-
 		INFO_LOG(COMMON, "ChunkReader: Writing %s" , _rFilename.c_str());
-		pFile = fopen(_rFilename.c_str(), "wb");
-		if (!pFile) {
+		File::IOFile pFile(_rFilename, "wb");
+		if (!pFile)
+		{
 			ERROR_LOG(COMMON,"ChunkReader: Error opening file for write");
 			return false;
 		}
@@ -281,20 +273,17 @@ public:
 		header.ExpectedSize = (int)sz;
 		
 		// Write to file
-		if (fwrite(&header, sizeof(SChunkHeader), 1, pFile) != 1) {
+		if (!pFile.WriteArray(&header, 1))
+		{
 			ERROR_LOG(COMMON,"ChunkReader: Failed writing header");
 			return false;
 		}
 
-		if (fwrite(buffer, sz, 1, pFile) != 1) {
+		if (!pFile.WriteBytes(buffer, sz))
+		{
 			ERROR_LOG(COMMON,"ChunkReader: Failed writing data");
 			return false;
 		}
-
-		if (fclose(pFile)) {
-			ERROR_LOG(COMMON,"ChunkReader: Error closing file! might be corrupted!");
-			return false;
-		}
 		
 		INFO_LOG(COMMON,"ChunkReader: Done writing %s", 
 				 _rFilename.c_str());

Source/Core/Common/Src/FileUtil.cpp

@@ -700,4 +700,108 @@ bool ReadFileToString(bool text_file, const char *filename, std::string &str)
 	return true;
 }
 
+IOFile::IOFile()
+	: m_file(NULL), m_good(true)
+{}
+
+IOFile::IOFile(std::FILE* file)
+	: m_file(file), m_good(true)
+{}
+
+IOFile::IOFile(const std::string& filename, const char openmode[])
+	: m_file(NULL), m_good(true)
+{
+	Open(filename, openmode);
+}
+
+IOFile::~IOFile()
+{
+	Close();
+}
+
+bool IOFile::Open(const std::string& filename, const char openmode[])
+{
+	Close();
+#ifdef _WIN32
+	fopen_s(&m_file, filename.c_str(), openmode);
+#else
+	m_file = fopen(filename.c_str(), openmode);
+#endif
+
+	m_good = IsOpen();
+	return m_good;
+}
+
+bool IOFile::Close()
+{
+	if (!IsOpen() || 0 != std::fclose(m_file))
+		m_good = false;
+
+	m_file = NULL;
+	return m_good;
+}
+
+std::FILE* IOFile::ReleaseHandle()
+{
+	std::FILE* const ret = m_file;
+	m_file = NULL;
+	return ret;
+}
+
+void IOFile::SetHandle(std::FILE* file)
+{
+	Close();
+	Clear();
+	m_file = file;
+}
+
+u64 IOFile::GetSize()
+{
+	if (IsOpen())
+		return File::GetSize(m_file);
+	else
+		return 0;
+}
+
+bool IOFile::Seek(s64 off, int origin)
+{
+	if (!IsOpen() || 0 != fseeko(m_file, off, origin))
+		m_good = false;
+
+	return m_good;
+}
+
+u64 IOFile::Tell()
+{	
+	if (IsOpen())
+		return ftello(m_file);
+	else
+		return -1;
+}
+
+bool IOFile::Flush()
+{
+	if (!IsOpen() || 0 != std::fflush(m_file))
+		m_good = false;
+
+	return m_good;
+}
+
+bool IOFile::Resize(u64 size)
+{
+	if (!IsOpen() || 0 !=
+#ifdef _WIN32
+		// ector: _chsize sucks, not 64-bit safe
+		// F|RES: changed to _chsize_s. i think it is 64-bit safe
+		_chsize_s(_fileno(m_file), size)
+#else
+		// TODO: handle 64bit and growing
+		ftruncate(fileno(m_file), size)
+#endif
+	)
+		m_good = false;
+
+	return m_good;
+}
+
 } // namespace

Source/Core/Common/Src/FileUtil.h

@@ -18,6 +18,8 @@
 #ifndef _FILEUTIL_H_
 #define _FILEUTIL_H_
 
+#include <fstream>
+#include <cstdio>
 #include <string>
 #include <vector>
 #include <string.h>
@@ -140,6 +142,77 @@ std::string GetBundleDirectory();
 bool WriteStringToFile(bool text_file, const std::string &str, const char *filename);
 bool ReadFileToString(bool text_file, const char *filename, std::string &str);
 
+// simple wrapper for cstdlib file functions to
+// hopefully will make error checking easier
+// and make forgetting an fclose() harder
+class IOFile : NonCopyable
+{
+public:
+	IOFile();
+	IOFile(std::FILE* file);
+	IOFile(const std::string& filename, const char openmode[]);
+
+	~IOFile();
+
+	bool Open(const std::string& filename, const char openmode[]);
+	bool Close();
+
+	template <typename T>
+	bool ReadArray(T* data, size_t length)
+	{
+		if (!IsOpen() || length != std::fread(data, sizeof(T), length, m_file))
+			m_good = false;
+
+		return m_good;
+	}
+
+	template <typename T>
+	bool WriteArray(const T* data, size_t length)
+	{
+		if (!IsOpen() || length != std::fwrite(data, sizeof(T), length, m_file))
+			m_good = false;
+
+		return m_good;
+	}
+
+	bool ReadBytes(void* data, size_t length)
+	{
+		return ReadArray(reinterpret_cast<char*>(data), length);
+	}
+
+	bool WriteBytes(const void* data, size_t length)
+	{
+		return WriteArray(reinterpret_cast<const char*>(data), length);
+	}
+
+	bool IsOpen() { return NULL != m_file; }
+
+	// m_good is set to false when a read, write or other function fails
+	bool IsGood() {	return m_good; }
+	operator void*() { return m_good ? m_file : NULL; }
+
+	std::FILE* ReleaseHandle();
+
+	std::FILE* GetHandle() { return m_file; }
+
+	void SetHandle(std::FILE* file);
+
+	bool Seek(s64 off, int origin);
+	u64 Tell();
+	u64 GetSize();
+	bool Resize(u64 size);
+	bool Flush();
+
+	// clear error state
+	void Clear() { m_good = true; std::clearerr(m_file); }
+
+private:
+	IOFile& operator=(const IOFile&) /*= delete*/;
+
+	std::FILE* m_file;
+	bool m_good;
+};
+
 }  // namespace
 
 #endif

Source/Core/Common/Src/LogManager.cpp

@@ -83,7 +83,7 @@ LogManager::LogManager()
 	m_Log[LogTypes::MEMCARD_MANAGER]	= new LogContainer("MemCard Manager", "MemCard Manager");
 	m_Log[LogTypes::NETPLAY]			= new LogContainer("NETPLAY",		"Netplay");
 
-	m_fileLog = new FileLogListener(File::GetUserPath(F_MAINLOG_IDX));
+	m_fileLog = new FileLogListener(File::GetUserPath(F_MAINLOG_IDX).c_str());
 	m_consoleLog = new ConsoleListener();
 
 	for (int i = 0; i < LogTypes::NUMBER_OF_LOGS; ++i) {
@@ -176,21 +176,16 @@ void LogContainer::trigger(LogTypes::LOG_LEVELS level, const char *msg) {
 	}
 }
 
-FileLogListener::FileLogListener(std::string filename) {
-	m_filename = filename;
-	m_logfile = fopen(filename.c_str(), "a+");
+FileLogListener::FileLogListener(const char *filename)
+{
+	m_logfile.open(filename, std::ios::app);
 	setEnable(true);
 }
 
-FileLogListener::~FileLogListener() {
-	if (m_logfile)
-		fclose(m_logfile);
-}
-
-void FileLogListener::Log(LogTypes::LOG_LEVELS, const char *msg) {
+void FileLogListener::Log(LogTypes::LOG_LEVELS, const char *msg)
+{
 	if (!m_enable || !isValid())
 		return;
 
-   	fwrite(msg, strlen(msg) * sizeof(char), 1, m_logfile);
-	fflush(m_logfile);
+	m_logfile << msg << std::flush;
 }

Source/Core/Common/Src/LogManager.h

@@ -21,10 +21,10 @@
 #include "Log.h"
 #include "StringUtil.h"
 #include "Thread.h"
+#include "FileUtil.h"
 
 #include <vector>
 #include <string.h>
-#include <stdio.h>
 
 #define	MAX_MESSAGES 8000   
 #define MAX_MSGLEN  1024
@@ -40,8 +40,7 @@ public:
 
 class FileLogListener : public LogListener {
 public:
-	FileLogListener(std::string filename);
-	~FileLogListener();
+	FileLogListener(const char *filename);
 
 	void Log(LogTypes::LOG_LEVELS, const char *msg);
 
@@ -60,8 +59,7 @@ public:
 	const char *getName() const { return "file"; }
 
 private:
-	std::string m_filename;
-	FILE *m_logfile;
+	std::ofstream m_logfile;
 	bool m_enable;
 };
 

Source/Core/Common/Src/NandPaths.cpp

@@ -52,20 +52,14 @@ std::string CreateTitleContentPath(u64 _titleID)
 
 bool CheckTitleTMD(u64 _titleID)
 {
-	std::string TitlePath;
-	TitlePath = CreateTitleContentPath(_titleID) + "/title.tmd";
+	const std::string TitlePath = CreateTitleContentPath(_titleID) + "/title.tmd";
 	if (File::Exists(TitlePath))
 	{
-		FILE* pTMDFile = fopen(TitlePath.c_str(), "rb");
-		if(pTMDFile)
-		{
-			u64 TitleID = 0xDEADBEEFDEADBEEFULL;
-			fseeko(pTMDFile, 0x18C, SEEK_SET);
-			fread(&TitleID, 8, 1, pTMDFile);
-			fclose(pTMDFile);
-			if (_titleID == Common::swap64(TitleID))
-				return true;
-		}
+		File::IOFile pTMDFile(TitlePath, "rb");
+		u64 TitleID = 0;
+		pTMDFile.Seek(0x18C, SEEK_SET);
+		if (pTMDFile.ReadArray(&TitleID, 1) && _titleID == Common::swap64(TitleID))
+			return true;
 	}
 	INFO_LOG(DISCIO, "Invalid or no tmd for title %08x %08x", (u32)(_titleID >> 32), (u32)(_titleID & 0xFFFFFFFF));
 	return false;
@@ -73,19 +67,14 @@ bool CheckTitleTMD(u64 _titleID)
 
 bool CheckTitleTIK(u64 _titleID)
 {
-	std::string TikPath = Common::CreateTicketFileName(_titleID);	
+	const std::string TikPath = Common::CreateTicketFileName(_titleID);	
 	if (File::Exists(TikPath))
 	{
-		FILE* pTIKFile = fopen(TikPath.c_str(), "rb");
-		if(pTIKFile)
-		{
-			u64 TitleID = 0xDEADBEEFDEADBEEFULL;
-			fseeko(pTIKFile, 0x1dC, SEEK_SET);
-			fread(&TitleID, 8, 1, pTIKFile);
-			fclose(pTIKFile);
-			if (_titleID == Common::swap64(TitleID))
-				return true;
-		}
+		File::IOFile pTIKFile(TikPath, "rb");
+		u64 TitleID = 0;
+		pTIKFile.Seek(0x1dC, SEEK_SET);
+		if (pTIKFile.ReadArray(&TitleID, 1) && _titleID == Common::swap64(TitleID))
+			return true;
 	}
 	INFO_LOG(DISCIO, "Invalid or no tik for title %08x %08x", (u32)(_titleID >> 32), (u32)(_titleID & 0xFFFFFFFF));
 	return false;

Source/Core/Common/Src/SysConf.cpp

@@ -57,29 +57,32 @@ bool SysConf::LoadFromFile(const char *filename)
 		else
 			return false;
 	}
-	FILE* f = fopen(filename, "rb");
 
-	if (f == NULL)
-		return false;
-	bool result = LoadFromFileInternal(f);
-	if (result)
-		m_Filename = filename;
-	fclose(f);
-	return result;
+	File::IOFile f(filename, "rb");
+	if (f.IsOpen())
+	{
+		if (LoadFromFileInternal(f.ReleaseHandle()))
+		{
+			m_Filename = filename;
+			return true;
+		}
+	}
+
+	return false;
 }
 
-bool SysConf::LoadFromFileInternal(FILE *f)
+bool SysConf::LoadFromFileInternal(File::IOFile f)
 {
 	// Fill in infos
 	SSysConfHeader s_Header;
-	if (fread(&s_Header.version, sizeof(s_Header.version), 1, f) != 1) return false;
-	if (fread(&s_Header.numEntries, sizeof(s_Header.numEntries), 1, f) != 1) return false;
+	f.ReadArray(s_Header.version, 4);
+	f.ReadArray(&s_Header.numEntries, 1);
 	s_Header.numEntries = Common::swap16(s_Header.numEntries) + 1;
 
 	for (u16 index = 0; index < s_Header.numEntries; index++)
 	{
 		SSysConfEntry tmpEntry;
-		if (fread(&tmpEntry.offset, sizeof(tmpEntry.offset), 1, f) != 1) return false;
+		f.ReadArray(&tmpEntry.offset, 1);
 		tmpEntry.offset = Common::swap16(tmpEntry.offset);
 		m_Entries.push_back(tmpEntry);
 	}
@@ -89,52 +92,62 @@ bool SysConf::LoadFromFileInternal(FILE *f)
 			i < m_Entries.end() - 1; i++)
 	{
 		SSysConfEntry& curEntry = *i;
-		if (fseeko(f, curEntry.offset, SEEK_SET) != 0) return false;
+		f.Seek(curEntry.offset, SEEK_SET);
 
 		u8 description = 0;
-		if (fread(&description, sizeof(description), 1, f) != 1) return false;
+		f.ReadArray(&description, 1);
 		// Data type
 		curEntry.type = (SysconfType)((description & 0xe0) >> 5);
 		// Length of name in bytes - 1
 		curEntry.nameLength = (description & 0x1f) + 1;
 		// Name
-		if (fread(&curEntry.name, curEntry.nameLength, 1, f) != 1) return false;
+		f.ReadArray(curEntry.name, curEntry.nameLength);
 		curEntry.name[curEntry.nameLength] = '\0';
 		// Get length of data
 		curEntry.dataLength = 0;
 		switch (curEntry.type)
 		{
 		case Type_BigArray:
-			if (fread(&curEntry.dataLength, 2, 1, f) != 1) return false;
+			f.ReadArray(&curEntry.dataLength, 1);
 			curEntry.dataLength = Common::swap16(curEntry.dataLength);
 			break;
+
 		case Type_SmallArray:
-			if (fread(&curEntry.dataLength, 1, 1, f) != 1) return false;
+			{
+			u8 dlength = 0;
+			f.ReadBytes(&dlength, 1);
+			curEntry.dataLength = dlength;
 			break;
+			}
+
 		case Type_Byte:
 		case Type_Bool:
 			curEntry.dataLength = 1;
 			break;
+
 		case Type_Short:
 			curEntry.dataLength = 2;
 			break;
+
 		case Type_Long:
 			curEntry.dataLength = 4;
 			break;
+
 		default:
 			PanicAlertT("Unknown entry type %i in SYSCONF (%s@%x)!",
 				curEntry.type, curEntry.name, curEntry.offset);
 			return false;
+			break;
 		}
 		// Fill in the actual data
 		if (curEntry.dataLength)
 		{
 			curEntry.data = new u8[curEntry.dataLength];
-			if (fread(curEntry.data, curEntry.dataLength, 1, f) != 1) return false;
+			f.ReadArray(curEntry.data, curEntry.dataLength);
 		}
 	}
 
-	return true;
+	return f.IsGood();
 }
 
 // Returns the size of the item in file
@@ -294,88 +307,87 @@ void SysConf::GenerateSysConf()
 		m_Entries.push_back(items[i]);
 
 	File::CreateFullPath(m_FilenameDefault);
-	FILE *g = fopen(m_FilenameDefault.c_str(), "wb");
+	File::IOFile g(m_FilenameDefault, "wb");
 
 	// Write the header and item offsets
-	fwrite(&s_Header.version, sizeof(s_Header.version), 1, g);
-	fwrite(&s_Header.numEntries, sizeof(u16), 1, g);
-	for (int i = 0; i < 27; ++i)
+	g.WriteArray(&s_Header.version, 1);
+	g.WriteArray(&s_Header.numEntries, 1);
+	for (int i = 0; i != 27; ++i)
 	{
-		u16 tmp_offset = Common::swap16(items[i].offset);
-		fwrite(&tmp_offset, 2, 1, g);
+		const u16 tmp_offset = Common::swap16(items[i].offset);
+		g.WriteArray(&tmp_offset, 1);
 	}
 	const u16 end_data_offset = Common::swap16(current_offset);
-	fwrite(&end_data_offset, 2, 1, g);
+	g.WriteArray(&end_data_offset, 1);
 	
 	// Write the items
 	const u8 null_byte = 0;
-	for (int i = 0; i < 27; ++i)
+	for (int i = 0; i != 27; ++i)
 	{
 		u8 description = (items[i].type << 5) | (items[i].nameLength - 1);
-		fwrite(&description, sizeof(description), 1, g);
-		fwrite(&items[i].name, items[i].nameLength, 1, g);
+		g.WriteArray(&description, 1);
+		g.WriteArray(&items[i].name, items[i].nameLength);
 		switch (items[i].type)
 		{
 			case Type_BigArray:
 				{
-					u16 tmpDataLength = Common::swap16(items[i].dataLength);
-					fwrite(&tmpDataLength, 2, 1, g);
-					fwrite(items[i].data, items[i].dataLength, 1, g);
-					fwrite(&null_byte, 1, 1, g);
+					const u16 tmpDataLength = Common::swap16(items[i].dataLength);
+					g.WriteArray(&tmpDataLength, 1);
+					g.WriteBytes(items[i].data, items[i].dataLength);
+					g.WriteArray(&null_byte, 1);
 				}
 				break;
+
 			case Type_SmallArray:
-				fwrite(&items[i].dataLength, 1, 1, g);
-				fwrite(items[i].data, items[i].dataLength, 1, g);
-				fwrite(&null_byte, 1, 1, g);
+				g.WriteArray(&items[i].dataLength, 1);
+				g.WriteBytes(items[i].data, items[i].dataLength);
+				g.WriteBytes(&null_byte, 1);
 				break;
+
 			default:
-				fwrite(items[i].data, items[i].dataLength, 1, g);
+				g.WriteBytes(items[i].data, items[i].dataLength);
 				break;
 		}
 	}
 
 	// Pad file to the correct size
-	const u64 cur_size = File::GetSize(g); 
-	for (unsigned int i = 0; i < 16380 - cur_size; ++i)
-		fwrite(&null_byte, 1, 1, g);
+	const u64 cur_size = g.GetSize();
+	for (unsigned int i = 0; i != 16380 - cur_size; ++i)
+		g.WriteBytes(&null_byte, 1);
 
 	// Write the footer
-	const char footer[5] = "SCed";
-	fwrite(&footer, 4, 1, g);
+	g.WriteBytes("SCed", 4);
 
-	fclose(g);
 	m_Filename = m_FilenameDefault;
 }
 
 bool SysConf::SaveToFile(const char *filename)
 {
-	FILE *f = fopen(filename, "r+b");
-
-	if (f == NULL)
-		return false;
+	File::IOFile f(filename, "r+b");
 
 	for (std::vector<SSysConfEntry>::iterator i = m_Entries.begin();
 			i < m_Entries.end() - 1; i++)
 	{
 		// Seek to after the name of this entry
-		if (fseeko(f, i->offset + i->nameLength + 1, SEEK_SET) != 0) return false;
+		f.Seek(i->offset + i->nameLength + 1, SEEK_SET);
+
 		// We may have to write array length value...
 		if (i->type == Type_BigArray)
 		{
-			u16 tmpDataLength = Common::swap16(i->dataLength);
-			if (fwrite(&tmpDataLength, 2, 1, f) != 1) return false;
+			const u16 tmpDataLength = Common::swap16(i->dataLength);
+			f.WriteArray(&tmpDataLength, 1);
 		}
 		else if (i->type == Type_SmallArray)
 		{
-			if (fwrite(&i->dataLength, 1, 1, f) != 1) return false;
+			const u8 len = i->dataLength;
+			f.WriteArray(&len, 1);
 		}
+
 		// Now write the actual data
-		if (fwrite(i->data, i->dataLength, 1, f) != 1) return false;
+		f.WriteBytes(i->data, i->dataLength);
 	}
 
-	fclose(f);
-	return true;
+	return f.IsGood();
 }
 
 bool SysConf::Save()

Source/Core/Common/Src/SysConf.h

@@ -179,7 +179,7 @@ public:
 	bool Reload();
 
 private:
-	bool LoadFromFileInternal(FILE *f);
+	bool LoadFromFileInternal(File::IOFile f);
 	void GenerateSysConf();
 	void Clear();