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

Use SDL_GetError for sdl2::ttf::init instead of the last OS error #1348

Open
wants to merge 1 commit into
base: master
Choose a base branch
from

Conversation

sprocklem
Copy link

Previously, the code use the last OS error. However, not all errors returned by TTF_Init are OS errors, and the expected behaviour is that the caller instead use SDL_GetError when an error is encountered.

To limit breakage for existing callers the error is still returned as an io::Error, albeit one with io::ErrorKind::Other. It is worth noting that this does lead to an observable difference in behaviour for callers – the error has a different kind – although any code that is inspecting this was already relying on an unreliable source of data.

This fixes issue #1347.

@sprocklem sprocklem marked this pull request as draft November 4, 2023 20:29
@sprocklem sprocklem changed the title Fix sdl2::ttf::init incorrectly incorrectly uses the last OS error Fix "sdl2::ttf::init incorrectly incorrectly uses the last OS error" Nov 4, 2023
@sprocklem sprocklem marked this pull request as ready for review November 4, 2023 20:32
@sprocklem sprocklem changed the title Fix "sdl2::ttf::init incorrectly incorrectly uses the last OS error" Use SDL_GetError for sdl2::ttf::init instead of the last OS error Nov 4, 2023
Previously, the code use the last OS error. However, not all errors
returned by `TTF_Init` are OS errors, and the expected behaviour is that
the caller instead use `SDL_GetError` when an error is encountered.

To limit breakage for existing application the error is still returned
as an `io::Error`, albeit one with `io::ErrorKind::Other`. It is
worth noting that this does lead to an observable difference in
behaviour for callers – the error has a different kind – although any
code that is inspecting this was already relying on an unreliable source
of data.

This fixes issue Rust-SDL2#1347.
@Cobrand
Copy link
Member

Cobrand commented Nov 9, 2023

To limit breakage for existing callers the error is still returned as an io::Error, albeit one with io::ErrorKind::Other. It is worth noting that this does lead to an observable difference in behaviour for callers – the error has a different kind – although any code that is inspecting this was already relying on an unreliable source of data.

I think it's better to break the API here and return the correct type instead (and put in in the changelog as breaking change). That will allow users that actually use it to change their code accordingly, rather than just breaking silently if they did use this before.

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