Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

[WIP] GCMemcard: Header verification. #8020

Open
wants to merge 6 commits into
base: master
from

Conversation

4 participants
@AdmiralCurtiss
Copy link
Contributor

commented Apr 21, 2019

Depends on #8019. This is the followup verification code for #7975.

Adds a bunch of sanity checks to identify possibly corrupted memory cards.

TODO:

  • Are all of these checks sensible?
  • Can we think of further checks?
  • Properly integrate the Dir and BAT checks into the GCMemcard constructor.
  • Proper error messages for the different kind of errors; possibly allow some kind of errors (data in unused area) but not others (wrong checksum or encoding).
  • GCMemcard::TestChecksums() is obsolete with this.
if (m_size_mb != card_size_mbits)
return ValidityCheckErrorCode::INVALID_CARD_SIZE;

// only known valid values are 0 for western and 1 for shift-jis

This comment has been minimized.

Copy link
@JosJuice

JosJuice Apr 21, 2019

Contributor

Are you sure? According to https://bugs.dolphin-emu.org/issues/10247 (see also the corresponding PR), Shift-JIS is not necessarily 1.

This comment has been minimized.

Copy link
@AdmiralCurtiss

AdmiralCurtiss Apr 21, 2019

Author Contributor

All my Japanese memory cards have a 1 there, but huh, interesting. Maybe anything nonzero counts then. e: Or maybe it's just bit 0? Probably should do some hardware tests.

This comment has been minimized.

Copy link
@Antidote

Antidote Apr 27, 2019

According to all the research I've done Shift-JIS isn't explicitly checked, only Western, so Shift-JIS would be implicitly non-zero.

Officially the SDK defines it as 1.

@@ -1287,7 +1376,7 @@ s32 GCMemcard::FZEROGX_MakeSaveGameValid(const Header& cardheader, const DEntry&
return 0;

// get encrypted destination memory card serial numbers
cardheader.CARD_GetSerialNo(&serial1, &serial2);
auto[serial1, serial2] = cardheader.CalculateSerial();

This comment has been minimized.

Copy link
@Antidote

Antidote Apr 28, 2019

Every open source implementation I've seen get's this very wrong, the serial number isn't two 32 bit values, it's a 64bit value. I know this because of how MP1 utilizes 64bit values for it's collision materials and the behavior is identical to this:
https://gitlab.axiodl.com/AxioDL/kabufuda/blob/master/lib/kabufuda/Card.cpp#L725-733

This comment has been minimized.

Copy link
@shuffle2

shuffle2 Apr 28, 2019

Contributor

I can't see how it matters, it's likely just the result of 64bit variable being lowered to gc ppc.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
You can’t perform that action at this time.