search

libsdl-org / SDL

Simple Directmedia Layer
https://libsdl.org
zlib License
8.37k stars 1.56k forks source link

`SDL_SetPropertyWithCleanup` doesn't call the cleanup function on failure #9642

Closed Susko3 closed 1 week ago

Susko3 commented 2 weeks ago

The docs say:

https://github.com/libsdl-org/SDL/blob/3f2f712fffcfa07dbba48e88aff24006ab012f21/include/SDL3/SDL_properties.h#L151-L152

The current implementation is as follows:

https://github.com/libsdl-org/SDL/blob/3f2f712fffcfa07dbba48e88aff24006ab012f21/src/SDL_properties.c#L341-L359

Firstly, calling SDL_SetPropertyWithCleanup with value: NULL will not call the cleanup function. This is technically not in violation of the documentation. But it is strange that you can provide a cleanup function knowing that it'll never be called.

More importantly, if the SDL_calloc fails, the cleanup function (for the value that is being set) will never be called.

SDL_PrivateSetProperty also doesn't clean up on failure.

The documentation could be wrong. But I can't think of a reason why you wouldn't want the cleanup function to clean up on failure.

slouken commented 2 weeks ago

Feel free to submit a PR to fix the code issues, thanks!