Add hack to IPCHLE to make NeoGamma work.

[magumagu]

NeoGamma is explicitly sending a nonsense command to the Bluetooth module;
make sure to respond with something sane.

Fixes issue 9470, a regression from PR #1856.

Yes, it's a bit hacky, but @JMC47 encouraged me to submit this. Feedback from anyone with a better understanding of how error handling works on IOS would be welcome.

---

This change is

[JMC47]

I still think this is a better behavior than it being undocumented in the code and then crashing.

[lioncash]

Review status: 0 of 1 files reviewed at latest revision, 1 unresolved discussion.

---

Source/Core/Core/IPC_HLE/WII_IPC_HLE_Device_usb.cpp, line 204 [r1] (raw file):
Brace should be on a new line.

---

Comments from Reviewable

[degasus]

Why does this differ from the "default" clase of the next switch/case?

[mimimi085181]

Thanks, it seems to work now. But i think it uncovers another issue, GetCoverStatus does not return that no disc is inserted, instead it times out. This might be related to the hack, but it might also be a general issue with Dolphin. On hardware, NeoGamma returns error 0x01023A00, if no disc is inserted, which translates into 0x01XXXXXX = "No disc inserted" and 0xXX023A00 = "Medium not present / Cover opened" according to NeoGamma, and according to YACD it means "lid open" and "Medium not present / Cover opened".
http://hitmen.c02.at/files/yagcd/yagcd/chap5.html#sec5.7

Could this explain some issues with the wii menu that are related to disc changes?

[BhaaLseN]

I just hacked together a homebrew and ran it on my Wii; that one simply returns -4 as error when I pass command 3 or higher (if I didn't mess up anywhere).

[mimimi085181]

Can you post the source code and the .dol of this, so magumagu can check if that is really what he means?

[BhaaLseN]

Here you go. And the Dol. Just bear in mind that its copy+paste from a different copy+pasted and hackish solution. I didn't verify the rest of the buffer tho, so I'm not sure if anything else should happen there (yet).

@degasus: I suppose it breaks during parsing inside the SIOCtlVBuffer, which would have to be guarded against in case of invalid data. But that's probably harder than just discarding the invalid data right away before parsing anything.

[magumagu]

@BhaaLseN Okay, -4 makes sense, I guess.

@degasus The SIOCtlVBuffer constructor can crash (maybe that shouldn't be the case, but it's not obvious how to fix that). Also, the existing default case doesn't send a reply.

@mimimi085181 Err, check again? I see a timeout, but it has nothing to do with GetCoverStatus.

[mimimi085181]

Yeah, i need to check that again, it's either resetting the drive or reading the disc id that times out. Gecko OS does not require this hack, but crashes with no disc inserted, so i guess there's at least 1 unrelated issue at play here.

[JMC47]

If we have a test saying -4 is what it should return, then is it really a hack if we did it like that? Or am I missing something.

[BhaaLseN]

-4 is the error returned by the IOCtlv call, which is IPC_EINVAL in libogc (error input value). The IOCtlv call shouldn't accept those bogus parameters, and instead return that.

[magumagu]

Updated with a less hacky version. I think this behaves correctly.

[BhaaLseN]

Suppose thats good enough (and it gets rid of another existing hack). At least on WiiBrew, theres no known IOCtl documented (only IOCtlv).
Longer term, its probably not a bad idea to validate and safeguard all IOCtl(v) we have and return EINVAL appropriately - the question is whether this pays off anywhere (since we haven't hit those with official games, only with Homebrew).

LGTM unless someone else finds a reason to keep the IOCtl active.

[JMC47]

Seems to perform fine for me.

[degasus]

Reviewed 1 of 1 files at r2.
Review status: all files reviewed at latest revision, 1 unresolved discussion.

---

Comments from Reviewable