From a6649da088ab79435d6b4f85fab27a4426d8d9f4 Mon Sep 17 00:00:00 2001
From: =?UTF-8?q?L=C3=A9o=20Lam?= <leo@innovatetechnologi.es>
Date: Mon, 20 Feb 2017 22:59:52 +0100
Subject: [PATCH] IOS/FFSP: Fix the read handler logic

This changes the read request handler to work just like IOS:

* To make things clearer, we now return early from error conditions,
  instead of having nested ifs.

* IOS does an additional check on the requested read length, and
  substracts the current seek position from it, if the read would
  cause IOS to read past the EOF (not sure what the purpose of this
  check is, but IOS does it, so we should too).

* The most significant one: IOS does *not* return the requested read
  length, or update the file seek position with it. Instead, it uses
  the *actual* read length.

As a result of simply doing what IOS does, this fixes _Mushroom Men_.
The game creates a save file, reads 2560 bytes from it, then
immediately writes 16384 bytes to it. With IOS, the first read does not
change the seek position at all, so the save data is written at
offset 0, not 2560. With Dolphin, the read erroneously set the
seek position to 2560, which caused the data to be written at
the wrong location.

Behavior confirmed by comparing IPC replies with IOS LLE and by looking
at the FS module in IOS.
---
 Source/Core/Core/IOS/FS/FileIO.cpp | 57 +++++++++++++++---------------
 1 file changed, 28 insertions(+), 29 deletions(-)

diff --git a/Source/Core/Core/IOS/FS/FileIO.cpp b/Source/Core/Core/IOS/FS/FileIO.cpp
index a277431e5a..7ab778747b 100644
--- a/Source/Core/Core/IOS/FS/FileIO.cpp
+++ b/Source/Core/Core/IOS/FS/FileIO.cpp
@@ -193,40 +193,39 @@ IPCCommandResult FileIO::Seek(const SeekRequest& request)
 
 IPCCommandResult FileIO::Read(const ReadWriteRequest& request)
 {
-  s32 return_value = FS_EACCESS;
-  if (m_file->IsOpen())
+  if (!m_file->IsOpen())
   {
-    if (m_Mode == IOS_OPEN_WRITE)
-    {
-      WARN_LOG(IOS_FILEIO, "FileIO: Attempted to read 0x%x bytes to 0x%08x on a write-only file %s",
-               request.size, request.buffer, m_name.c_str());
-    }
-    else
-    {
-      DEBUG_LOG(IOS_FILEIO, "FileIO: Read 0x%x bytes to 0x%08x from %s", request.size,
-                request.buffer, m_name.c_str());
-      m_file->Seek(m_SeekPos, SEEK_SET);  // File might be opened twice, need to seek before we read
-      return_value = static_cast<u32>(
-          fread(Memory::GetPointer(request.buffer), 1, request.size, m_file->GetHandle()));
-      if (static_cast<u32>(return_value) != request.size && ferror(m_file->GetHandle()))
-      {
-        return_value = FS_EACCESS;
-      }
-      else
-      {
-        m_SeekPos += request.size;
-      }
-    }
-  }
-  else
-  {
-    ERROR_LOG(IOS_FILEIO, "FileIO: Failed to read from %s (Addr=0x%08x Size=0x%x) - file could "
+    ERROR_LOG(IOS_FILEIO, "Failed to read from %s (Addr=0x%08x Size=0x%x) - file could "
                           "not be opened or does not exist",
               m_name.c_str(), request.buffer, request.size);
-    return_value = FS_ENOENT;
+    return GetDefaultReply(FS_ENOENT);
   }
 
-  return GetDefaultReply(return_value);
+  if (m_Mode == IOS_OPEN_WRITE)
+  {
+    WARN_LOG(IOS_FILEIO, "Attempted to read 0x%x bytes to 0x%08x on a write-only file %s",
+             request.size, request.buffer, m_name.c_str());
+    return GetDefaultReply(FS_EACCESS);
+  }
+
+  u32 requested_read_length = request.size;
+  // IOS has this check in the read request handler.
+  if (requested_read_length + m_SeekPos > static_cast<u32>(m_file->GetSize()))
+    requested_read_length -= m_SeekPos;
+
+  DEBUG_LOG(IOS_FILEIO, "Read 0x%x bytes to 0x%08x from %s", request.size, request.buffer,
+            m_name.c_str());
+  m_file->Seek(m_SeekPos, SEEK_SET);  // File might be opened twice, need to seek before we read
+  const u32 number_of_bytes_read = static_cast<u32>(
+      fread(Memory::GetPointer(request.buffer), 1, requested_read_length, m_file->GetHandle()));
+
+  if (number_of_bytes_read != request.size && ferror(m_file->GetHandle()))
+    return GetDefaultReply(FS_EACCESS);
+
+  // IOS returns the number of bytes read and adds that value to the seek position,
+  // instead of adding the *requested* read length.
+  m_SeekPos += number_of_bytes_read;
+  return GetDefaultReply(number_of_bytes_read);
 }
 
 IPCCommandResult FileIO::Write(const ReadWriteRequest& request)