Skip to content

Conversation

@eloj
Copy link
Contributor

@eloj eloj commented Oct 19, 2025

This may have already been discussed and rejected before, in which case I apologize for the noise, but I got inspired by a recent change merged to mesa.

This branch contains the result of applying a coccinelle spatch to remove basic NULL checks before SDL_free():

@@
expression ptr;
@@

-if (ptr) {
-SDL_free(ptr);
-}
+SDL_free(ptr);

@@
expression ptr;
@@

-if (ptr != NULL) {
-SDL_free(ptr);
-}
+SDL_free(ptr);

I also 'manually' did the same for free(), but there were only three occurences.

@slouken
Copy link
Collaborator

slouken commented Oct 19, 2025

Hmm, we probably don't want the coccinelle patch, but otherwise, @icculus, how do you feel about this?

@slouken
Copy link
Collaborator

slouken commented Oct 19, 2025

Actually, POSIX explicitly states that free(NULL) does nothing, so I think I'm okay with this.

Copy link
Collaborator

@icculus icculus left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

This is fine. I clean these out when I find them, so it's nice to have it all done at once.

We don't need the coccinelle patch.

eloj added 2 commits October 20, 2025 08:38
Replaces the pattern

  if (ptr) {
    SDL_free(ptr);
  }

with

  SDL_free(ptr);
@eloj eloj marked this pull request as ready for review October 20, 2025 07:05
@eloj
Copy link
Contributor Author

eloj commented Oct 20, 2025

Rebased and ready to go. (The RISCOS build failed to bring up the container.)

@1bsyl
Copy link
Contributor

1bsyl commented Oct 20, 2025

maybe we can keep the patch, so that it can be run once in a while

@slouken slouken merged commit 3b0347a into libsdl-org:main Oct 20, 2025
42 of 43 checks passed
@slouken
Copy link
Collaborator

slouken commented Oct 20, 2025

Merged, thanks!

@eloj
Copy link
Contributor Author

eloj commented Oct 20, 2025

That was my original intent by including it @1bsyl, that maybe you'd want to make it e.g part of the checklist for releasing a major version. I didn't actually hook/link it into anything because I didn't want to add work for maintainers out of the blue.

The spatch is trivial, but I pasted it into the top post for posterity.

@eloj eloj deleted the nocheck-sdlfree branch October 20, 2025 07:22
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.

5 participants