Use portable file reads for DLC texture data

Minecraft.Client/Textures/Packs/DLCTexturePack.cpp

@@ -8,14 +8,47 @@
 #include "../../Platform/Common/DLC/DLCTextureFile.h"
 #include "../../Platform/Common/DLC/DLCLocalisationFile.h"
 #include "../../../Minecraft.World/Util/StringHelpers.h"
+#include "../../../Minecraft.World/Util/PortableFileIO.h"
 #include "../../Utils/StringTable.h"
 #include "../../Platform/Common/DLC/DLCAudioFile.h"
 
+#include <limits>
+
 #if defined _XBOX || defined _WINDOWS64
 #include "../../Platform/Xbox/XML/ATGXmlParser.h"
 #include "../../Platform/Xbox/XML/xmlFilesCallback.h"
 #endif
 
+namespace
+{
+	bool ReadPortableBinaryFile(File &file, PBYTE &data, DWORD &size)
+	{
+		const __int64 fileLength = file.length();
+		if (fileLength < 0 || fileLength > static_cast<__int64>(std::numeric_limits<DWORD>::max()))
+		{
+			data = NULL;
+			size = 0;
+			return false;
+		}
+
+		const std::size_t capacity = static_cast<std::size_t>(fileLength);
+		PBYTE buffer = new BYTE[capacity == 0 ? 1 : capacity];
+		const PortableFileIO::BinaryReadResult readResult = PortableFileIO::ReadBinaryFile(file.getPath(), buffer, capacity);
+		if (readResult.status != PortableFileIO::BinaryReadStatus::ok
+			|| readResult.fileSize > std::numeric_limits<DWORD>::max())
+		{
+			delete [] buffer;
+			data = NULL;
+			size = 0;
+			return false;
+		}
+
+		data = buffer;
+		size = static_cast<DWORD>(readResult.fileSize);
+		return true;
+	}
+}
+
 DLCTexturePack::DLCTexturePack(DWORD id, DLCPack *pack, TexturePack *fallback) : AbstractTexturePack(id, NULL, pack->getName(), fallback)
 {
 	m_dlcInfoPack = pack;
@@ -305,30 +338,12 @@ int DLCTexturePack::packMounted(LPVOID pParam,int iPad,DWORD dwErr,DWORD dwLicen
 
 				if(xzpPath.exists())
 				{
-					const char *pchFilename=wstringtofilename(xzpPath.getPath());
+					PBYTE pbData = NULL;
-					HANDLE fileHandle = CreateFile(
+					DWORD bytesRead = 0;
-						pchFilename, // file name
+					if( ReadPortableBinaryFile(xzpPath, pbData, bytesRead) )
-						GENERIC_READ, // access mode
-						0, // share mode // TODO 4J Stu - Will we need to share file? Probably not but...
-						NULL, // Unused
-						OPEN_EXISTING , // how to create // TODO 4J Stu - Assuming that the file already exists if we are opening to read from it
-						FILE_FLAG_SEQUENTIAL_SCAN, // file attributes
-						NULL // Unsupported
-						);
-
-					if( fileHandle != INVALID_HANDLE_VALUE )
 					{
-						DWORD dwFileSize = xzpPath.length();
+						DLCUIDataFile *uiDLCFile = (DLCUIDataFile *)texturePack->m_dlcDataPack->addFile(DLCManager::e_DLCType_UIData,L"TexturePack.xzp");
-						DWORD bytesRead;
+						uiDLCFile->addData(pbData,bytesRead,true);
-						PBYTE pbData =  (PBYTE) new BYTE[dwFileSize];
-						BOOL success = ReadFile(fileHandle,pbData,dwFileSize,&bytesRead,NULL);
-						CloseHandle(fileHandle);
-						if(success)
-						{
-							DLCUIDataFile *uiDLCFile = (DLCUIDataFile *)texturePack->m_dlcDataPack->addFile(DLCManager::e_DLCType_UIData,L"TexturePack.xzp");
-							uiDLCFile->addData(pbData,bytesRead,true);
-						
-						}
 					}
 				}
 #else
@@ -354,43 +369,10 @@ int DLCTexturePack::packMounted(LPVOID pParam,int iPad,DWORD dwErr,DWORD dwLicen
 							File grf( getFilePath(texturePack->m_dlcInfoPack->GetPackID(), dlcFile->getGrfPath() ) );
 							if (grf.exists())
 							{
-#if defined(_UNICODE) && !defined(__linux__)
+								PBYTE pbData = NULL;
-								std::wstring path = grf.getPath();
+								DWORD dwFileSize = 0;
-								const WCHAR *pchFilename=path.c_str();
+								if( ReadPortableBinaryFile(grf, pbData, dwFileSize) )
-								HANDLE fileHandle = CreateFile(
-									pchFilename, // file name
-									GENERIC_READ, // access mode
-									0, // share mode // TODO 4J Stu - Will we need to share file? Probably not but...
-									NULL, // Unused
-									OPEN_EXISTING , // how to create // TODO 4J Stu - Assuming that the file already exists if we are opening to read from it
-									FILE_FLAG_SEQUENTIAL_SCAN, // file attributes
-									NULL // Unsupported
-									);
-#else
-								const char *pchFilename=wstringtofilename(grf.getPath());
-								HANDLE fileHandle = CreateFile(
-									pchFilename, // file name
-									GENERIC_READ, // access mode
-									0, // share mode // TODO 4J Stu - Will we need to share file? Probably not but...
-									NULL, // Unused
-									OPEN_EXISTING , // how to create // TODO 4J Stu - Assuming that the file already exists if we are opening to read from it
-									FILE_FLAG_SEQUENTIAL_SCAN, // file attributes
-									NULL // Unsupported
-									);
-#endif
-
-								if( fileHandle != INVALID_HANDLE_VALUE )
 								{
-									DWORD dwFileSize = grf.length();
-									DWORD bytesRead;
-									PBYTE pbData =  (PBYTE) new BYTE[dwFileSize];
-									BOOL bSuccess = ReadFile(fileHandle,pbData,dwFileSize,&bytesRead,NULL);
-									if(bSuccess==FALSE)
-									{
-										app.FatalLoadError();
-									}
-									CloseHandle(fileHandle);
-
 									// 4J-PB - is it possible that we can get here after a read fail and it's not an error?
 									dlcFile->setGrfData(pbData, dwFileSize, texturePack->m_stringTable);
 
@@ -398,6 +380,10 @@ int DLCTexturePack::packMounted(LPVOID pParam,int iPad,DWORD dwErr,DWORD dwLicen
 
 									app.m_gameRules.setLevelGenerationOptions( dlcFile->lgo );
 								}
+								else
+								{
+									app.FatalLoadError();
+								}
 							}
 						}
 					}
@@ -406,45 +392,17 @@ int DLCTexturePack::packMounted(LPVOID pParam,int iPad,DWORD dwErr,DWORD dwLicen
 						File grf(getFilePath(texturePack->m_dlcInfoPack->GetPackID(), levelGen->getBaseSavePath() ));
 						if (grf.exists())
 						{
-#if defined(_UNICODE) && !defined(__linux__)
+							PBYTE pbData = NULL;
-							std::wstring path = grf.getPath();
+							DWORD dwFileSize = 0;
-							const WCHAR *pchFilename=path.c_str();
+							if( ReadPortableBinaryFile(grf, pbData, dwFileSize) )
-							HANDLE fileHandle = CreateFile(
-								pchFilename, // file name
-								GENERIC_READ, // access mode
-								0, // share mode // TODO 4J Stu - Will we need to share file? Probably not but...
-								NULL, // Unused
-								OPEN_EXISTING , // how to create // TODO 4J Stu - Assuming that the file already exists if we are opening to read from it
-								FILE_FLAG_SEQUENTIAL_SCAN, // file attributes
-								NULL // Unsupported
-								);
-#else
-							const char *pchFilename=wstringtofilename(grf.getPath());
-							HANDLE fileHandle = CreateFile(
-								pchFilename, // file name
-								GENERIC_READ, // access mode
-								0, // share mode // TODO 4J Stu - Will we need to share file? Probably not but...
-								NULL, // Unused
-								OPEN_EXISTING , // how to create // TODO 4J Stu - Assuming that the file already exists if we are opening to read from it
-								FILE_FLAG_SEQUENTIAL_SCAN, // file attributes
-								NULL // Unsupported
-								);
-#endif
-
-							if( fileHandle != INVALID_HANDLE_VALUE )
 							{
-								DWORD bytesRead,dwFileSize = GetFileSize(fileHandle,NULL);
-								PBYTE pbData =  (PBYTE) new BYTE[dwFileSize];
-								BOOL bSuccess = ReadFile(fileHandle,pbData,dwFileSize,&bytesRead,NULL);
-								if(bSuccess==FALSE)
-								{
-									app.FatalLoadError();
-								}
-								CloseHandle(fileHandle);
-
 								// 4J-PB - is it possible that we can get here after a read fail and it's not an error?
 								levelGen->setBaseSaveData(pbData, dwFileSize);
 							}
+							else
+							{
+								app.FatalLoadError();
+							}
 						}
 					}
 				}

Minecraft.World/Util/PortableFileIO.h

@@ -1,8 +1,7 @@
 #pragma once
 
 #include <cstddef>
-#include <fstream>
+#include <cstdio>
-#include <ios>
 #include <string>
 
 #include "StringHelpers.h"
@@ -24,31 +23,72 @@ namespace PortableFileIO
 		std::size_t fileSize;
 	};
 
+	inline std::FILE *OpenBinaryFileForRead(const std::wstring &path)
+	{
+#if defined(_WIN32)
+		return _wfopen(path.c_str(), L"rb");
+#else
+		const std::string nativePath = wstringtofilename(path);
+		return std::fopen(nativePath.c_str(), "rb");
+#endif
+	}
+
+	inline bool Seek(std::FILE *file, std::size_t offset, int origin)
+	{
+#if defined(_WIN32)
+		return _fseeki64(file, static_cast<__int64>(offset), origin) == 0;
+#else
+		return fseeko(file, static_cast<off_t>(offset), origin) == 0;
+#endif
+	}
+
+	inline __int64 Tell(std::FILE *file)
+	{
+#if defined(_WIN32)
+		return _ftelli64(file);
+#else
+		return static_cast<__int64>(ftello(file));
+#endif
+	}
+
 	inline BinaryReadResult ReadBinaryFile(const std::wstring &path, void *buffer, std::size_t capacity)
 	{
-		const std::string nativePath = wstringtofilename(path);
+		std::FILE *stream = OpenBinaryFileForRead(path);
-		std::ifstream stream(nativePath.c_str(), std::ios::binary | std::ios::ate);
+		if (stream == NULL)
-		if (!stream.is_open())
 		{
 			return { BinaryReadStatus::not_found, 0, 0 };
 		}
 
-		const std::streamoff endPosition = stream.tellg();
+		if (!Seek(stream, 0, SEEK_END))
+		{
+			std::fclose(stream);
+			return { BinaryReadStatus::read_error, 0, 0 };
+		}
+
+		const __int64 endPosition = Tell(stream);
 		if (endPosition < 0)
 		{
+			std::fclose(stream);
 			return { BinaryReadStatus::read_error, 0, 0 };
 		}
 
 		const std::size_t fileSize = static_cast<std::size_t>(endPosition);
 		if (fileSize > capacity)
 		{
+			std::fclose(stream);
 			return { BinaryReadStatus::too_large, 0, fileSize };
 		}
 
-		stream.seekg(0, std::ios::beg);
+		if (!Seek(stream, 0, SEEK_SET))
-		stream.read(reinterpret_cast<char *>(buffer), static_cast<std::streamsize>(fileSize));
+		{
-		const std::size_t bytesRead = static_cast<std::size_t>(stream.gcount());
+			std::fclose(stream);
-		if ((!stream && !stream.eof()) || bytesRead != fileSize)
+			return { BinaryReadStatus::read_error, 0, fileSize };
+		}
+
+		const std::size_t bytesRead = std::fread(buffer, 1, fileSize, stream);
+		const bool failed = std::ferror(stream) != 0;
+		std::fclose(stream);
+		if (failed || bytesRead != fileSize)
 		{
 			return { BinaryReadStatus::read_error, bytesRead, fileSize };
 		}
@@ -58,29 +98,42 @@ namespace PortableFileIO
 
 	inline BinaryReadResult ReadBinaryFileSegment(const std::wstring &path, std::size_t offset, void *buffer, std::size_t bytesToRead)
 	{
-		const std::string nativePath = wstringtofilename(path);
+		std::FILE *stream = OpenBinaryFileForRead(path);
-		std::ifstream stream(nativePath.c_str(), std::ios::binary | std::ios::ate);
+		if (stream == NULL)
-		if (!stream.is_open())
 		{
 			return { BinaryReadStatus::not_found, 0, 0 };
 		}
 
-		const std::streamoff endPosition = stream.tellg();
+		if (!Seek(stream, 0, SEEK_END))
+		{
+			std::fclose(stream);
+			return { BinaryReadStatus::read_error, 0, 0 };
+		}
+
+		const __int64 endPosition = Tell(stream);
 		if (endPosition < 0)
 		{
+			std::fclose(stream);
 			return { BinaryReadStatus::read_error, 0, 0 };
 		}
 
 		const std::size_t fileSize = static_cast<std::size_t>(endPosition);
 		if ((offset > fileSize) || (bytesToRead > (fileSize - offset)))
 		{
+			std::fclose(stream);
 			return { BinaryReadStatus::too_large, 0, fileSize };
 		}
 
-		stream.seekg(static_cast<std::streamoff>(offset), std::ios::beg);
+		if (!Seek(stream, offset, SEEK_SET))
-		stream.read(reinterpret_cast<char *>(buffer), static_cast<std::streamsize>(bytesToRead));
+		{
-		const std::size_t bytesRead = static_cast<std::size_t>(stream.gcount());
+			std::fclose(stream);
-		if ((!stream && !stream.eof()) || bytesRead != bytesToRead)
+			return { BinaryReadStatus::read_error, 0, fileSize };
+		}
+
+		const std::size_t bytesRead = std::fread(buffer, 1, bytesToRead, stream);
+		const bool failed = std::ferror(stream) != 0;
+		std::fclose(stream);
+		if (failed || bytesRead != bytesToRead)
 		{
 			return { BinaryReadStatus::read_error, bytesRead, fileSize };
 		}