suloku / gcmm

A gamecube/wii memory card manager
GNU General Public License v3.0
255 stars 24 forks source link

There is a bug in __card_getfilenum #15

Closed GoogleCodeExporter closed 9 years ago

GoogleCodeExporter commented 9 years ago
__card_getfilenum uses strnicmp instead of simply stricmp, this causes 
incorrect behavior when two files have similar filenames e.g, gczelda (TWW) and 
gczelda2 (TP)

Here is a patch that corrects this behavior:
https://dl.dropboxusercontent.com/u/21757902/card_c_strnicmp.patch

Original issue reported on code.google.com by antidote.crk@gmail.com on 23 Aug 2013 at 8:42

GoogleCodeExporter commented 9 years ago
Thank you very much for noticing, I didn't have twilight princess for gamecube, 
so I guess I never got the chance of this happening to me.

I'll fix this in a couple hours.

ps: sorry for the late response, I saw this last week, but I didn't have time 
to correct the bug since today.

Btw, what is the incorrect behavior? I'm gonna test it, I'm curious to see what 
happens.

Original comment by sulokuTDCmago@gmail.com on 3 Sep 2013 at 8:46

GoogleCodeExporter commented 9 years ago
[deleted comment]
GoogleCodeExporter commented 9 years ago
I've been checking the code and even in the case when trying to open gczelda 
file from the memory card and the code stomps with gczelda2, then it checks the 
gamecode, and as it is different, it should proceed to the next file in the 
entri list...

So, what is exactly the incorrect behavior and how to reproduce it?

ps: still, I agree it is better to just use stricmp

Original comment by sulokuTDCmago@gmail.com on 3 Sep 2013 at 9:09

GoogleCodeExporter commented 9 years ago
A more thourough testing has revealed that the problem wasn't with the strnicmp 
function, but with gamecode detection, as I stated earlier it should prevent 
the behavior you experienced.

Your fix does indeed fix the bug, but the bug remains if you have 2 savegames 
of the same game but from different regions, so the problem is with gamecode 
detection.

I hope I can fix it soon.

Original comment by sulokuTDCmago@gmail.com on 3 Sep 2013 at 10:14

GoogleCodeExporter commented 9 years ago
Ok, I think I nailed it. I (and others before me) incorrectly assumed that 
CARD_init()can be used to set the company and the gamecode and that it can be 
called as many times as wanted to change it. But that is wrong, this function 
only works the first time, which makes sense, as a game will only use a single 
company and gamecode for its savefiles.

I just need to change all the card_init calls for Card_setcompany and 
card_setgamecode and card_getfilenum will work correctly, no need to change it 
(as it is part of libogc I'd prefer to leave it as unchanged as possible).

The program has been working because the first call to card_init is made with 
null params, so this allows to list all entries in the card. In fact I myself 
fixed savegame restoring (which sometimes worked and sometimes didn't) by using 
card_setcompany and card_setgamecode, and then I took on GCMM. Silly me I 
didn't notice then.

Original comment by sulokuTDCmago@gmail.com on 3 Sep 2013 at 11:34

GoogleCodeExporter commented 9 years ago
Done. I lost an mc in the process, I think it is because its old, as the 
problems appear with older versions of gcmm and only in this mc (not a nintendo 
MC).

_card_sectorerase failed whenever I tried to restore an image to that MC, I 
couldn't find out why.

But the filename/region bug is fixed. Thanks again for noticing.

Original comment by sulokuTDCmago@gmail.com on 3 Sep 2013 at 3:26

GoogleCodeExporter commented 9 years ago
That make sense, I wasn't using CARD_init to set the gamecode or company

Original comment by antidote.crk@gmail.com on 6 Sep 2013 at 5:36