Emulator

case 0x02: // m_pFileHandle = fopen(m_Filename.c_str(), "wb"); break;
// MK Wii gets here corrupting its saves, however using rb+
mode works fine
// TODO : figure it properly...
case 0x03: m_pFileHandle = fopen(m_Filename.c_str(), "r+b"); break;

you guys comment case 0x02 without break; in the end so it use case 0x03 instread

so i remove comment in case 0x02 and use mode a+b in case 0x03 and it work for this
game now

forget this info

i remove comment in case 0x02 use mode r+b
and use mode a+b in case 0x03

work fine

Well, I suggest you try your fix on other games to make sure there isn't any side
effects.

well It has side effect The save game can corrupt in some game.
I wonder if it can switch r+b ,a+b, wb or other with some type of file such as
system,save or something

now I use a+r mode only on .aar file (which I don't know what is it)
and r+b mode (same old mode) on other type of file and it work at least 10 games I
have

std::string typeCheck;
int numPos;
.
.
.
case 0x02: m_pFileHandle = fopen(m_Filename.c_str(), "r+b"); break;
// MK Wii gets here corrupting its saves, however using rb+
mode works fine
// TODO : figure it properly...
case 0x03: typeCheck = m_Filename.c_str();
numPos = (int)typeCheck.find(".aar");

				if( numPos != (int)std::string::npos ) 
				{
					m_pFileHandle = fopen

(m_Filename.c_str(), "a+b");
//PanicAlert("file %s",
m_Filename.c_str());
}
else
m_pFileHandle = fopen
(m_Filename.c_str(), "r+b");

				break;

Very interesting.
How about changing 0x02 to r+b, 0x03 to w+b?

a+b work for that .aar file in one piece

r+b works for other type of file
w+b works too but I don't know if it will delete system file or save file

**Only a+b work for that .aar file

if use r+b game not load

I'd suggest you add some DEBUG_LOGs (or ERROR_LOGs, to keep the log unspammed) that
keep track of stuff; including m_pFileHandle (regardless of when its zero or not) and
the _Mode parameter for Open.

_Mode 0x01 should probably never call Write, while 0x02 should never do a Read (which
means it should be rb and wb/ab respectively)

if you see the code you will know that mode 0x02 has comment // and no break; in the
end then it used r+b same as 0x03 for very long time.

so i remove comment // and use r+b in 0x02

and change 0x03 to a+b , well the game load perfect but after close/save game system
files were all corrupted.

after that I use PanicAlert to see all the file that use mode 0x03. then I see
some .dat some save and some .aar file which create in /tmp/ .I decided to use mode
a+b only on .aar file which all the name of files are kinda look like temp file.

and now this work perfect no system/save corrupted anymore

just wonder if some other games maybe require a+b in some type of file .and how I
gonna take care of it.

Could you try r4684?

sorry to say it does not work

How about this patch?

...don't work

Well, I must say unless I have that game, I don't know what happened there.

yes I think so.

Issue 3356 has been merged into this issue.

hello i compiled dolphin whith my modified WII_IPC_HLE_Device_FileIO.cpp
and it worck finely

i want to become a committer

Issue 1489 has been merged into this issue.

Issue 2337 has been merged into this issue.

Issue 2592 has been merged into this issue.

Issue 2785 has been merged into this issue.

Please respond if this issue is still valid, or it will be closed.

Yes, this issue is still valid. There is a problem with the NAND emulation where some files need to be opened using a+b.

ah yeah um...this issue shouldn't have been hit by my script :/
probably just something we can steal from sneek.

test with this patch pls http://pastie.org/private/ejefmtiozaidkyps4cuw

ZTP wii and Metroid prime 3 are games that are also likely affected by this patch

One Piece UC 2 still hangs at the loading screen after applying the patch in comment 29.

Someone might wanna try this one: http://pastie.org/private/b1blwyrg9hwddqgcpw4a
I'm not exactly sure about ::Open tho, i think 1 should be "rb", 2 should be "wb" (its write-only after all) and 3 should either be "r+b" or "w+b" (still trying to find out whether it should create files or not).

j4ck.fr0st, your patch didn't fix One Piece UC 2 either.

I don't have UC, but those seek modes are closer to what SNEEK does (and apparently more correct).
Open still does odd things if you ask me (like what, IOS_OPEN_WRITE does r+b?), so just fixing one of them isnt gonna work.

Next one: http://pastie.org/1389986
I wrote a small homebrew and tested Open/Write/Read/Seek on my wii, and with this patch Dolphin acts the same way.
I don't have One Piece UC (or MK Wii, which also seems to be affected by save corruption), so please test this one.

One Piece 2 still does not work. I have attached a log of what happens. The filenames being loaded (.aar) are repeating and the seek position is not increasing. The hack patch in Comment 5 uses a+b for the .aar files to get the game to load.

Now that is interesting. See the GETFILESTATS ioctl between the two seek+writes to a .aar file? What if it seeks to what GETFILESTATS returns as length - which is 0 in that case, but shouldn't, as the file size has increased after the first write.

Well, it's most probably due to using File::GetSize to read the size, which I suspect uses stat to come by the file size, which wouldn't correctly reflect an unclosed/unflushed file's size.

So what one may want to do instead is use a clever ftello+fseeko+ftello+fseeko combination to read the file's size or otherwise keep track of the correct m_FileLength all along while Writing (m_FileLength=max(m_FileLength,ftello+WrittenSize)).

With "m_FileLength=max(m_FileLength,ftello+WrittenSize)" I meant using ftello before the write and getting the written size after the write, which is actually quite silly. Better use ftello after the write and then "m_FileLength=max(m_FileLength,ftello)"

Try this: https://gist.github.com/748611

I stole and adapted some changes from j4ck.fr0st, I hope you don't mind ;)

Primarily, it does two things: One, it removes the seek_position mod file_length stuff, so you can seek to the end of file again and two, it removes fetching the current file length ISFS_IOCTL_GETFILESTATS from the file system (which is wrong semantics in that case) and instead keeps track and updates the current file length when writing.

That log sounds interresting, gonna look at it later. However, my changes should be seen as the correct way, this was verified against real Wii hardware.
I did consider using the internal variable m_Seek and increment/reset it accordingly on Seek/Read/Write, but then i noticed how it was used, and thought it wasnt required. Maybe a simple flush after writing will do the trick?

@j4ck.fr0st: Yeah, I reckon this should work, too, but consider the performance impact flushing after every write would have (especially if a game does a lot of small consecutive writes).

Maybe better flush before fetching the file size inside the ISFS_IOCTL_GETFILESTATS (before the File::GetSize() call of course), but I don't see what's wrong with tracking the file size internally, it looks to me like the correct semantics.

Oh, and I forgot to mention that this fixes One Piece: Unlimited Cruise (1 and 2) for me. I can't say anything about if it affects other games (positively or negatively) since I don't have those others that were mentioned in connection with this or similar issues.

I'm usually a fan of reinventing wheels, but I don't think we should do that here...

How about we "fix" File::GetSize(FILE*) to do the tell/seek/tell/seek thingy, rather than calling the other overload with fileno?
http://pastie.org/1392594 (same patch as before, except including a changed FileUtil.cpp)

blubberdiblub, you now have commit access to the Dolphin repository to commit whichever patch you feel is best.

Did anyone test mine? I'd guess it should solve the problem, unless something else is pretty wrong there.
Or do any other games suffer from this, preferably games I have?

With either patch mkwiiv still corrupts its save if writeonly opens the file as wb

I will commit most of our changes, but without the "wb" change, as soon as I find out why committing doesn't work for me ;)

blubber: You have to check out from https, not just http in order to be able to commit.

lp: do you have a log of DiscIO when MKWii corrupts the saves? Write-only is supposed to open the file in some sort of write mode, and i'm pretty sure its supposed to be wb.

I'll throw some homebrew at my Wii and see if ab makes sense, but i kinda doubt that.

Just tried it out, ISFS_OPEN_WRITE sets the pointer to 0x00 (at least thats what ISFS_Seek(fd, 0, SEEK_CUR) returns)

logs attached
rksys.dat is the corrupted file
file is opened with ISFS_OPEN_WRITE writes a 2800KB file all 0x0
file is closed, reopened with ISFS_OPEN_WRITE writes 160KB of data and the game works until the next launch
(this is opening wb)
with the current code the file is identical other than the 2640KB of zero padding at the end

What if ISFS_OPEN_WRITE on a real Wii - while opening the file write only - doesn't truncate the file? Could someone able to debug on a Wii please check this? j4ck.fr0st?

If that were the case, fopen'ing it "wb" wouldn't be correct, since that truncates the file. Neither would "r+b" be correct, since that allows reading (but doesn't truncate the file and so gets quite near to the ISFS_OPEN_WRITE-doesn't-truncate semantics).

If it really turns out that we need to open the file for writing without truncating, a simple fopen() won't do. But we could help ourselves with open() + fdopen(). I already consulted the manpage of fdopen and it says that using "w" (or "wb", respectively) here doesn't truncate the file.

fwiw mario kart doesnt try to read from the file while it is in ISFS_OPEN_WRITE mode

blubber: ISFS_OPEN_WRITE does not truncate the file. Seek(SEEK_CUR) returns 0 after opening, Seek(SEEK_END) returns my filesize.
Sounds like a good point, and our only chances with fopen are to either keep the r+w hack, or use ab plus an additional fseek call to reposition at the beginning.

lp: Your log still shows 0x000000 as filesize, does it change with r6634?
If not, does this patch make a difference? http://pastie.org/1396101

@lpfaint: Yeah, and that's probably why MK Wii is working fine with "r+b" in ISFS_OPEN_WRITE mode.

But I agree with j4ck.fr0st that there might be games/apps trying that, and if the real Wii doesn't allow reading in that case, dolphin mustn't either.

@j4ck.fr0st: No, we cannot use any of the "a" modes. When a file is in append mode, is doesn't merely position the file pointer at eof when opening the file. Instead it does so before every write call. It would break all games that seek to a point inside the file (other than EOF) in order to write there.

log is from r6634 except ISFS_OPEN_WRITE is set to wb
the patch that changes it to ab causes the 160KB to be written after the 2800KB of 0

log is showing 0 as filesize because we use the m_filesize which is only updated on open or ISFS_IOCTL_GETFILESTATS calls

Yeah, I opted for not fetching the file size after every write, in order not to impact performance due to unneccessary seeking in the file. If you reckon this wouldn't hurt in any case and on any OS, I guess there would be nothing wrong with putting it after the fwrites instead.

we probably dont need the filesize as part of the seek log, just another part of the hack that was removed from seek

Yep, I think so.

Please try this: https://gist.github.com/748611/ae8306f961b57c5c376e8a568dd0c8c6a067dbcd

It's kinda ugly, but it should indeed open the file just for writing without truncating it.

needs #include <io.h> for windows but it does work correctly for mario kart

looks ok...feel free to remove commented sections if they no longer apply...that is something version control can handle :)
besides, it is probably identical to how IOS actually works with file descriptors

Still it's quite ugly. I'd prefer sticking to fopen(..., "r+b") until someone comes up with a better idea. It shouldn't break that way as long as no game wants to run into a error-because-read-not-allowed situation deliberately for some obscure purpose.

I could only test that patch with what I had available, and thats basically the homebrew I wrote for testing. And it seems you never cease to learn new things, I believed ab to simply position the write pointer at EOF, but not to reposition it automagically before every write.

I'd prefer a clean solution, especially now that you said "as long as no game wants to run into it" - you can be sure that we'll get another report because of that soon :P
Taking shortcuts is fine with me, but not when they sacrifice a lot of accuracy and/or look like some chimpansee took a dump on the repository.

blubber: Would you be more satisfied if we just did the open/fdopen game for all of them, not just write? It would be at least more consistent than mixing them together, and if i got that right, it does more/less accurately the same as real hardware.

Derived from/based on blubbers code: http://pastie.org/1410953
I asked skid to test ZTP Wii, but apparently he couldn't reproduce anything of Issue 3671. MK Wii should still be fine with it, can anyone please test it?

Using my patch is not advised, further testing proved it to be troublesome for certain operations. In particular, it fked up my SYSCONF a couple of times - open/fdopen doesnt seem to be a good solution either.

The only thing left would be keeping track of the open mode, and raise errors accordingly by ourselves: http://pastie.org/1421188
Someone with ZTP Wii please test this one; MK Wii seems to be fine?

Issue 3356 has been merged into this issue.