From 6ed4431d8baae235d90a400a6fdcaffcfe83d3db Mon Sep 17 00:00:00 2001
From: Lioncash <mathew1800@gmail.com>
Date: Wed, 15 Apr 2020 14:21:22 -0400
Subject: [PATCH] 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.
---
 src/common/file_util.cpp | 30 ++++++++++++++++++++++++++++++
 src/common/file_util.h   | 17 +++++------------
 2 files changed, 35 insertions(+), 12 deletions(-)

diff --git a/src/common/file_util.cpp b/src/common/file_util.cpp
index fd1a3fd30..3cae15526 100644
--- a/src/common/file_util.cpp
+++ b/src/common/file_util.cpp
@@ -992,6 +992,36 @@ bool IOFile::Flush() {
     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) {
     if (!IsOpen() || 0 !=
 #ifdef _WIN32
diff --git a/src/common/file_util.h b/src/common/file_util.h
index d5915b8b7..b7cbfdafd 100644
--- a/src/common/file_util.h
+++ b/src/common/file_util.h
@@ -273,12 +273,7 @@ public:
         static_assert(std::is_trivially_copyable_v<T>,
                       "Given array does not consist of trivially copyable objects");
 
-        if (!IsOpen()) {
-            m_good = false;
-            return std::numeric_limits<std::size_t>::max();
-        }
-
-        std::size_t items_read = std::fread(data, sizeof(T), length, m_file);
+        std::size_t items_read = ReadImpl(data, length, sizeof(T));
         if (items_read != length)
             m_good = false;
 
@@ -290,12 +285,7 @@ public:
         static_assert(std::is_trivially_copyable_v<T>,
                       "Given array does not consist of trivially copyable objects");
 
-        if (!IsOpen()) {
-            m_good = false;
-            return std::numeric_limits<std::size_t>::max();
-        }
-
-        std::size_t items_written = std::fwrite(data, sizeof(T), length, m_file);
+        std::size_t items_written = WriteImpl(data, length, sizeof(T));
         if (items_written != length)
             m_good = false;
 
@@ -349,6 +339,9 @@ public:
     }
 
 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();
 
     std::FILE* m_file = nullptr;