From 4e414902bec27bc8fb7af476bedf7974c1f35ef9 Mon Sep 17 00:00:00 2001 From: Lioncash Date: Fri, 6 Sep 2019 03:23:42 -0400 Subject: [PATCH 1/3] MemoryWriter: Use a std::unique_ptr for FILE handle in save() Prevents potential leaks from occurring within failure cases (e.g. with the default exception handler). --- src/athena/MemoryWriter.cpp | 18 ++++++++++-------- 1 file changed, 10 insertions(+), 8 deletions(-) diff --git a/src/athena/MemoryWriter.cpp b/src/athena/MemoryWriter.cpp index 4e8bb61..256cbf9 100644 --- a/src/athena/MemoryWriter.cpp +++ b/src/athena/MemoryWriter.cpp @@ -185,11 +185,11 @@ void MemoryWriter::save(std::string_view filename) { return; } - if (!filename.empty()) + if (!filename.empty()) { m_filepath = filename; + } - FILE* out = fopen(m_filepath.c_str(), "wb"); - + std::unique_ptr out{std::fopen(m_filepath.c_str(), "wb"), std::fclose}; if (!out) { atError(fmt("Unable to open file '{}'"), m_filepath); setError(); @@ -200,22 +200,24 @@ void MemoryWriter::save(std::string_view filename) { atUint64 blocksize = BLOCKSZ; do { - if (blocksize > m_length - done) + if (blocksize > m_length - done) { blocksize = m_length - done; + } - atInt64 ret = fwrite(m_data + done, 1, blocksize, out); + const atInt64 ret = std::fwrite(m_data + done, 1, blocksize, out.get()); if (ret < 0) { atError(fmt("Error writing data to disk")); setError(); return; - } else if (ret == 0) + } + + if (ret == 0) { break; + } done += blocksize; } while (done < m_length); - - fclose(out); } void MemoryWriter::writeUBytes(const atUint8* data, atUint64 length) { From 57ad780321c798f29dfd375e8fc46b7e9de385ec Mon Sep 17 00:00:00 2001 From: Lioncash Date: Fri, 6 Sep 2019 03:25:40 -0400 Subject: [PATCH 2/3] MemoryWriter: Remove unnecessary type cast within MemoryWriter constructor and setData() The input type is already the same type as the class member, so the cast is unnecessary. While we're at it, we can also remove an unnecessary initializer for m_position, since we initialize this to zero within the class declaration already. --- src/athena/MemoryWriter.cpp | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-) diff --git a/src/athena/MemoryWriter.cpp b/src/athena/MemoryWriter.cpp index 256cbf9..a7e3c68 100644 --- a/src/athena/MemoryWriter.cpp +++ b/src/athena/MemoryWriter.cpp @@ -10,7 +10,7 @@ namespace athena::io { MemoryWriter::MemoryWriter(atUint8* data, atUint64 length, bool takeOwnership) -: m_data((atUint8*)data), m_length(length), m_position(0), m_bufferOwned(takeOwnership) { +: m_data(data), m_length(length), m_bufferOwned(takeOwnership) { if (!data) { atError(fmt("data cannot be NULL")); setError(); @@ -156,7 +156,7 @@ void MemoryWriter::setData(atUint8* data, atUint64 length, bool takeOwnership) { if (m_bufferOwned) delete m_data; - m_data = (atUint8*)data; + m_data = data; m_length = length; m_position = 0; m_bufferOwned = takeOwnership; From de45f0896feeb257b2a6d9b41fdbab9ddadb3cfb Mon Sep 17 00:00:00 2001 From: Lioncash Date: Fri, 6 Sep 2019 03:28:29 -0400 Subject: [PATCH 3/3] MemoryWriter: Use a std::unique_ptr within resize() Same behavior, but keeps the memory managed throughout its whole lifetime. --- src/athena/MemoryWriter.cpp | 13 ++++++------- 1 file changed, 6 insertions(+), 7 deletions(-) diff --git a/src/athena/MemoryWriter.cpp b/src/athena/MemoryWriter.cpp index a7e3c68..607a964 100644 --- a/src/athena/MemoryWriter.cpp +++ b/src/athena/MemoryWriter.cpp @@ -260,15 +260,14 @@ void MemoryCopyWriter::resize(atUint64 newSize) { } // Allocate and copy new buffer - atUint8* newArray = new atUint8[newSize]; - memset(newArray, 0, newSize); - - if (m_dataCopy) - memmove(newArray, m_dataCopy.get(), m_length); - m_dataCopy.reset(newArray); + auto newArray = std::make_unique(newSize); + if (m_dataCopy) { + std::memmove(newArray.get(), m_dataCopy.get(), m_length); + } + m_dataCopy = std::move(newArray); // Swap the pointer and size out for the new ones. - m_data = newArray; + m_data = m_dataCopy.get(); m_length = newSize; }