SFML / CSFML

Official binding of SFML for C
https://www.sfml-dev.org
Other
342 stars 121 forks source link

Use `assert` to protect against dereferencing null pointers #287

Closed ChrisThrasher closed 1 week ago

ChrisThrasher commented 1 week ago

Closes #239

This pull request changes CSFML to use SFML's policy for assertions. In short, if a user-provided argument is able to invoke UB, we assert against that potential value. In switching to CSFML to this pattern, we get a few benefits.

  1. Errors are much more visible. Instead of returning a dummy value and logging, we instead terminate the program. Program termination is hard to ignore. This will lead to users of CSFML more quickly finding and fixing their errors.
  2. The implementation is easier to read. These macros from Internal.hpp aren't too hard to understand but it's certainly easier to read an idiomatic assert plus a normal line of code.
  3. More implementation flexibility. These macros assumed the pointer contained a member named This. Now that the macros are gone we can rename those members or pick different implementation techniques like what I proposed in #222 for example.
  4. Less logging. With Internal.hpp gone CSFML now no longer directly uses sf::err or any other logging utilities. I prefer libraries not do logging without a way to intercept and redirect those logs which CSFML currently does not have.

CSFML as it stands prior to this PR defines the behavior of null pointer arguments in Debug builds while leaving that behavior undefined for Release builds so that aspect of CSFML has not changed. All this PR does is change what that Debug behavior is.