Skip to content

Fix LocaleIterator for platforms where c_char is u8 (better fix?) #1479

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

Open
wants to merge 5 commits into
base: master
Choose a base branch
from

Conversation

geniot
Copy link

@geniot geniot commented Apr 20, 2025

I had the same problem as the previous pull-requester when compiling on aarch64:

error[E0308]: mismatched types
  --> /root/.cargo/git/checkouts/rust-sdl2-4bcf6005ccaf1a39/ecd03de/src/sdl2/locale.rs:78:33
   |
78 |     let region = try_get_string(sdl_locale.country);
   |                  -------------- ^^^^^^^^^^^^^^^^^^ expected `*const i8`, found `*const u8`
   |                  |
   |                  arguments to this function are incorrect
   |
   = note: expected raw pointer `*const i8`
              found raw pointer `*const u8`
note: function defined here
  --> /root/.cargo/git/checkouts/rust-sdl2-4bcf6005ccaf1a39/ecd03de/src/sdl2/locale.rs:86:11
   |
86 | unsafe fn try_get_string(ptr: *const i8) -> Option<String> {
   |           ^^^^^^^^^^^^^^ --------------

error[E0308]: mismatched types
  --> /root/.cargo/git/checkouts/rust-sdl2-4bcf6005ccaf1a39/ecd03de/src/sdl2/locale.rs:90:39
   |
90 |         Some(std::ffi::CStr::from_ptr(ptr).to_string_lossy().into_owned())
   |              ------------------------ ^^^ expected `*const u8`, found `*const i8`
   |              |
   |              arguments to this function are incorrect
   |
   = note: expected raw pointer `*const u8`
              found raw pointer `*const i8`
note: associated function defined here
  --> /rustc/05f9846f893b09a1be1fc8560e33fc3c815cfecb/library/core/src/ffi/c_str.rs:264:25

Anyway, I believe my fix is a better one... Choose one :)

@Cobrand
Copy link
Member

Cobrand commented Apr 21, 2025

I do think this fix is better than the other one, but please add an entry to the changelog because this will definitely break those already using this with aarch64.

@geniot
Copy link
Author

geniot commented Apr 21, 2025

I do think this fix is better than the other one, but please add an entry to the changelog because this will definitely break those already using this with aarch64.

Change log updated.

What do you mean "this will definitely break those already using this with aarch64" ?
On the contrary, this change fixes SDL2 usage on aarch64.

@Cobrand
Copy link
Member

Cobrand commented Apr 22, 2025

Ah right my bad, disregard my comment about the breaking change then, I see now it can't compile on aarch64 without this fix, I thought it compiled but the output type was not user-friendly.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants