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.
This commit is contained in:
Léo Lam 2017-02-20 22:59:52 +01:00
parent 5814d23fd9
commit a6649da088

View File

@ -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)