file_util: Early-exit in WriteArray and ReadArray if specified lengths are zero

It's undefined behavior to pass a null pointer to std::fread and
std::fwrite, even if the length passed in is zero, so we must perform
the precondition checking ourselves.

A common case where this can occur is when passing in the data of an
empty std::vector and size, as an empty vector will typically have a
null internal buffer.

While we're at it, we can move the implementation out of line and add
debug checks against passing in nullptr to std::fread and std::fwrite.
This commit is contained in:
Lioncash 2020-04-15 14:21:22 -04:00 committed by FearlessTobi
parent c605bb42db
commit 6ed4431d8b
2 changed files with 35 additions and 12 deletions

View File

@ -992,6 +992,36 @@ bool IOFile::Flush() {
return m_good; return m_good;
} }
std::size_t IOFile::ReadImpl(void* data, std::size_t length, std::size_t data_size) {
if (!IsOpen()) {
m_good = false;
return std::numeric_limits<std::size_t>::max();
}
if (length == 0) {
return 0;
}
DEBUG_ASSERT(data != nullptr);
return std::fread(data, data_size, length, m_file);
}
std::size_t IOFile::WriteImpl(const void* data, std::size_t length, std::size_t data_size) {
if (!IsOpen()) {
m_good = false;
return std::numeric_limits<std::size_t>::max();
}
if (length == 0) {
return 0;
}
DEBUG_ASSERT(data != nullptr);
return std::fwrite(data, data_size, length, m_file);
}
bool IOFile::Resize(u64 size) { bool IOFile::Resize(u64 size) {
if (!IsOpen() || 0 != if (!IsOpen() || 0 !=
#ifdef _WIN32 #ifdef _WIN32

View File

@ -273,12 +273,7 @@ public:
static_assert(std::is_trivially_copyable_v<T>, static_assert(std::is_trivially_copyable_v<T>,
"Given array does not consist of trivially copyable objects"); "Given array does not consist of trivially copyable objects");
if (!IsOpen()) { std::size_t items_read = ReadImpl(data, length, sizeof(T));
m_good = false;
return std::numeric_limits<std::size_t>::max();
}
std::size_t items_read = std::fread(data, sizeof(T), length, m_file);
if (items_read != length) if (items_read != length)
m_good = false; m_good = false;
@ -290,12 +285,7 @@ public:
static_assert(std::is_trivially_copyable_v<T>, static_assert(std::is_trivially_copyable_v<T>,
"Given array does not consist of trivially copyable objects"); "Given array does not consist of trivially copyable objects");
if (!IsOpen()) { std::size_t items_written = WriteImpl(data, length, sizeof(T));
m_good = false;
return std::numeric_limits<std::size_t>::max();
}
std::size_t items_written = std::fwrite(data, sizeof(T), length, m_file);
if (items_written != length) if (items_written != length)
m_good = false; m_good = false;
@ -349,6 +339,9 @@ public:
} }
private: private:
std::size_t ReadImpl(void* data, std::size_t length, std::size_t data_size);
std::size_t WriteImpl(const void* data, std::size_t length, std::size_t data_size);
bool Open(); bool Open();
std::FILE* m_file = nullptr; std::FILE* m_file = nullptr;