search

rust3ds / ctru-rs

Rust wrapper for libctru
https://rust3ds.github.io/ctru-rs/
Other
117 stars 17 forks source link

Mii Selector returns garbage result in Ok #121

Closed DeltaF1 closed 1 year ago

DeltaF1 commented 1 year ago

On my device, running examples/mii-selector.rs produces a popup window that says "There are no selectable miis" and "Press (A) to go back". Pressing A closes the popup but the program continues without panicking at let result = mii_selector.launch().unwrap();, and instead it appears that an empty Box is interpreted as valid MiiData instead. is_mii_selected is false, but the program still uses the MiiData object. I believe this is UB in safe code (pointer to garbage data). For example it says the Mii is of type Guest and the "name" field of the Guest variant is shown as a bunch of \0 null chars. The checksum test doesn't fix this because the CRC of all zeroes is also 0.

Suggestion

One of the following changes:

  1. MiiSelector::launch changed to return Result<Option<SelectionResult>, LaunchError>
  2. Add a variant Cancelled to LaunchError

In either case, is_mii_selected can be removed/deprecated (as it should always be true if a SelectionResult has been created!).

DeltaF1 commented 1 year ago

Okay I made this too hastily without reading more of the code, sorry. I still think needing to check is_mii_selected is not a great UX and could lead to coding errors, and making a change to the API would be nice. But I see the data isn't totally garbage since it's using Default.

Techie-Pi commented 1 year ago

Seems like a good idea, when I wrote the MiiSelector code I just made a 1-to-1 translation of the C struct. But this would definitively be more idiomatic.

Meziu commented 1 year ago

Under what circumstances did this error appear? I thought that the user had to have at least one Mii to use the console. Maybe this happened in an emulator?

Either way, if there are edge cases in which the return is incorrect, we can just check if a Mii was selected and return an error otherwise (a new variant of LaunchErroris the best way).

DeltaF1 commented 1 year ago

Maybe this happened in an emulator?

This is on my real New 3DS XL. The mii-selector.rs example calls blacklist_user_mii and since I don't have any Guest Miis there are none to select. Removing this call from the example lets me select my user Mii.

Enabling the option "MII_SELECTOR_CANCEL" and pressing the cancel button causes the same behaviour (empty MiiData is returned). I don't see a way to distinguish between "user cancelled" and "no miis available" in the return data, so probably only a single variant needs to be added. I'd like to make a PR for this.

Meziu commented 1 year ago

Great, we gladly accept contributions.