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

udev: Clear intermediate errors when attempting to load libudev.so #8419

Closed
wants to merge 1 commit into from

Conversation

mgorny
Copy link

@mgorny mgorny commented Oct 22, 2023

Description

Call SDL_ClearError() after a successful SDL_LoadObject() call to clear the previous error left over by the previous SDL_UDEV_load_syms() procedure (set by SDL_LoadFunction()). This ensures that no error is set when SDL_UDEV_Init() returns successfully.

SDL_UDEV_LoadLibrary() first attempts to find udev symbols in the libraries that are already loaded, then attempts to find and load libudev.so* using a list of names. Every unsuccessful call to SDL_UDEV_load_syms() leaves the error set. However, if a subsequent call succeeds (i.e. after loading the library), the previous error remains set until SDL_UDEV_Init() returns.

This is a serious problem in SDL2 since the (stale) error leaks to the caller after:

SDL_Init(SDL_INIT_GAMECONTROLLER);

In SDL3, the issue is still present but the error is incidentally cleared by the call to PLATFORM_hid_init().

Call `SDL_ClearError()` after a successful `SDL_LoadObject()` call
to clear the previous error left over by the previous
`SDL_UDEV_load_syms()` procedure (set by `SDL_LoadFunction()`).  This
ensures that no error is set when `SDL_UDEV_Init()` returns
successfully.

`SDL_UDEV_LoadLibrary()` first attempts to find udev symbols
in the libraries that are already loaded, then attempts to find
and load `libudev.so*` using a list of names.  Every unsuccessful call
to `SDL_UDEV_load_syms()` leaves the error set.  However,
if a subsequent call succeeds (i.e. after loading the library),
the previous error remains set until `SDL_UDEV_Init()` returns.

This is a serious problem in SDL2 since the (stale) error leaks
to the caller after:

    SDL_Init(SDL_INIT_GAMECONTROLLER);

In SDL3, the issue is still present but the error is incidentally
cleared by the call to `PLATFORM_hid_init()`.
@slouken
Copy link
Collaborator

slouken commented Oct 22, 2023

I don't mind accepting this, but in general you should only call SDL_GetError() when the API level function returns an error. There are many places where internal functions can generate an error that is resolvable by the higher level API, and we only guarantee that the error message is correct when the API returns an error.

After thinking about this, it may be something we want to change for SDL3, as a global pass.

@slouken slouken closed this Oct 22, 2023
@slouken slouken mentioned this pull request Oct 22, 2023
@chewi
Copy link
Contributor

chewi commented Oct 22, 2023

@slouken I worked with @mgorny on figuring this out. In case it wasn't clear from the description, SDL_UDEV_Init() never works under SDL2 without this change unless you LD_PRELOAD libudev first. The first call to SDL_UDEV_load_syms() fails, then the second call succeeds, but the SDL_UDEV_LoadLibrary() still thinks it failed because the error hasn't been cleared.

Never mind, I now realise it does work, it's just the error flag is set. I previously thought the error was fatal.

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.

None yet

3 participants