SDL3: SDL_assert should have optional error message

[dimhotepus]

Common SDL_assert usage pattern is to check error codes like https://github.com/libsdl-org/SDL/blob/1ba99c53d48ec1f1f7c58b20326fc0c964ce5aa2/src/thread/pthread/SDL_sysmutex.c#L73-74:

const int rc = pthread_mutex_lock(&mutex->id);
SDL_assert(rc == 0);

It would be nice to dump rc here, like:

SDL_assert(rc == 0, "pthread_mutex_lock failed w/e %d: %s", rc, strerror(rc));

Also (may be different issue) mutex lock can fail but app doesn't know for sure. See https://github.com/libsdl-org/SDL/blob/1ba99c53d48ec1f1f7c58b20326fc0c964ce5aa2/src/thread/pthread/SDL_sysmutex.c#L61 Suggest to return at least SDL_bool here so app can check. Or we expect to use SDL_TryLockMutex for this?

[icculus]

That specific assert was to check that locking a mutex can never fail, but as we just saw, one can destroy a mutex and use the free'd memory to trigger this.

So either this assert is wrong because an app can trigger it, or we did the app a massive favor by leaving it here. :)

So we should decide if we want a fancier assert (which would need to allocate memory, so maybe not) before ABI lock, and also what to do with this specific assert.

[icculus]

My thinking is this specific assert could just change to...

SDL_assert((rc == 0) && "pthread_mutex_lock failed! Did you accidentally destroy this SDL_Mutex?");

...if we're keeping it.

[dimhotepus]

Guess we can use buffer on the stack and truncate output if needed. It is debug thing, no problem if it is truncated.

What do you think?

[icculus]

The info lives in a global linked list once an assert fires, for later recovery by the program, and lives as static memory at each use of SDL_assert, so it becomes expensive to add a buffer.

Maybe we could add a macro that allows formatting for just specific asserts, though.

[slouken]

This is something we can add later via a differently named macro, so I'm moving it out of the ABI milestone.

[slouken]

We are scoping work for the SDL 3.2.0 release, so please let us know if this is a showstopper for you.

[dimhotepus]

Nope, not a showstopper.