UICommon/ResourcePack: Use ScopeGuards to manage closing files

Makes it way harder to introduce resource leaks, and plugs the existing
resource leaks in the constructor and Install() where the file wouldn't
be closed in some error cases.
This commit is contained in:
Lioncash 2019-05-27 13:02:01 -04:00
parent 157a305507
commit 41cda6fe6d
1 changed files with 9 additions and 13 deletions

View File

@ -10,6 +10,7 @@
#include "Common/FileSearch.h" #include "Common/FileSearch.h"
#include "Common/FileUtil.h" #include "Common/FileUtil.h"
#include "Common/ScopeGuard.h"
#include "Common/StringUtil.h" #include "Common/StringUtil.h"
#include "UICommon/ResourcePack/Manager.h" #include "UICommon/ResourcePack/Manager.h"
@ -23,35 +24,34 @@ constexpr char TEXTURE_PATH[] = "Load/Textures/";
// this ourselves // this ourselves
static bool ReadCurrentFileUnlimited(unzFile file, std::vector<char>& destination) static bool ReadCurrentFileUnlimited(unzFile file, std::vector<char>& destination)
{ {
const uint32_t MAX_BUFFER_SIZE = 65535; const u32 MAX_BUFFER_SIZE = 65535;
if (unzOpenCurrentFile(file) != UNZ_OK) if (unzOpenCurrentFile(file) != UNZ_OK)
return false; return false;
uint32_t bytes_to_go = static_cast<uint32_t>(destination.size()); Common::ScopeGuard guard{[&] { unzCloseCurrentFile(file); }};
auto bytes_to_go = static_cast<u32>(destination.size());
while (bytes_to_go > 0) while (bytes_to_go > 0)
{ {
int bytes_read = unzReadCurrentFile(file, &destination[destination.size() - bytes_to_go], const int bytes_read = unzReadCurrentFile(file, &destination[destination.size() - bytes_to_go],
std::min(bytes_to_go, MAX_BUFFER_SIZE)); std::min(bytes_to_go, MAX_BUFFER_SIZE));
if (bytes_read < 0) if (bytes_read < 0)
{ {
unzCloseCurrentFile(file);
return false; return false;
} }
bytes_to_go -= bytes_read; bytes_to_go -= static_cast<u32>(bytes_read);
} }
unzCloseCurrentFile(file);
return true; return true;
} }
ResourcePack::ResourcePack(const std::string& path) : m_path(path) ResourcePack::ResourcePack(const std::string& path) : m_path(path)
{ {
auto file = unzOpen(path.c_str()); auto file = unzOpen(path.c_str());
Common::ScopeGuard file_guard{[&] { unzClose(file); }};
if (file == nullptr) if (file == nullptr)
{ {
@ -129,8 +129,6 @@ ResourcePack::ResourcePack(const std::string& path) : m_path(path)
m_textures.push_back(filename.substr(9)); m_textures.push_back(filename.substr(9));
} while (unzGoToNextFile(file) != UNZ_END_OF_LIST_OF_FILE); } while (unzGoToNextFile(file) != UNZ_END_OF_LIST_OF_FILE);
unzClose(file);
} }
bool ResourcePack::IsValid() const bool ResourcePack::IsValid() const
@ -172,6 +170,7 @@ bool ResourcePack::Install(const std::string& path)
} }
auto file = unzOpen(m_path.c_str()); auto file = unzOpen(m_path.c_str());
Common::ScopeGuard file_guard{[&] { unzClose(file); }};
for (const auto& texture : m_textures) for (const auto& texture : m_textures)
{ {
@ -229,10 +228,7 @@ bool ResourcePack::Install(const std::string& path)
out.flush(); out.flush();
} }
unzClose(file);
SetInstalled(*this, true); SetInstalled(*this, true);
return true; return true;
} }