Fix double-free in SDL_CreateRendererWithProperties()

[Sackzement]

Warning

[ 35%] Building C object CMakeFiles/SDL3-shared.dir/src/render/SDL_render.c.o
SDL/src/render/SDL_render.c:1142:13: warning: Attempt to free released memory [clang-analyzer-unix.Malloc]
 1142 |             SDL_free(driver_error);
      |             ^
SDL/include/SDL3/SDL_stdinc.h:6023:18: note: expanded from macro 'SDL_free'
 6023 | #define SDL_free free
      |                  ^
SDL/src/render/SDL_render.c:1320:17: note: Assuming 'surface' is non-null
 1320 |     CHECK_PARAM(!surface) {
      |                 ^
SDL/src/SDL_internal.h:302:34: note: expanded from macro 'CHECK_PARAM'
  302 | #define CHECK_PARAM(invalid) if (invalid)
      |                                  ^~~~~~~
SDL/src/render/SDL_render.c:1320:5: note: Taking false branch
 1320 |     CHECK_PARAM(!surface) {
      |     ^
SDL/src/SDL_internal.h:302:30: note: expanded from macro 'CHECK_PARAM'
  302 | #define CHECK_PARAM(invalid) if (invalid)
      |                              ^
SDL/src/render/SDL_render.c:1327:16: note: Calling 'SDL_CreateRendererWithProperties'
 1327 |     renderer = SDL_CreateRendererWithProperties(props);
      |                ^~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~
SDL/src/render/SDL_render.c:1051:17: note: Assuming 'window' is non-null
 1051 |     CHECK_PARAM(!window && !surface && (!driver_name || SDL_strcmp(driver_name, SDL_GPU_RENDERER) != 0)) {
      |                 ^
SDL/src/SDL_internal.h:302:34: note: expanded from macro 'CHECK_PARAM'
  302 | #define CHECK_PARAM(invalid) if (invalid)
      |                                  ^~~~~~~
SDL/src/render/SDL_render.c:1051:25: note: Left side of '&&' is false
 1051 |     CHECK_PARAM(!window && !surface && (!driver_name || SDL_strcmp(driver_name, SDL_GPU_RENDERER) != 0)) {
      |                         ^
SDL/src/render/SDL_render.c:1056:17: note: 'window' is non-null
 1056 |     CHECK_PARAM(window && surface) {
      |                 ^
SDL/src/render/SDL_render.c:1056:17: note: Left side of '&&' is true
SDL/src/render/SDL_render.c:1056:27: note: Assuming 'surface' is null
 1056 |     CHECK_PARAM(window && surface) {
      |                           ^
SDL/src/render/SDL_render.c:1056:5: note: Taking false branch
 1056 |     CHECK_PARAM(window && surface) {
      |     ^
SDL/src/SDL_internal.h:302:30: note: expanded from macro 'CHECK_PARAM'
  302 | #define CHECK_PARAM(invalid) if (invalid)
      |                              ^
SDL/src/render/SDL_render.c:1061:17: note: 'window' is non-null
 1061 |     CHECK_PARAM(window && SDL_WindowHasSurface(window)) {
      |                 ^
SDL/src/render/SDL_render.c:1061:17: note: Left side of '&&' is true
SDL/src/render/SDL_render.c:1061:27: note: Assuming the condition is false
 1061 |     CHECK_PARAM(window && SDL_WindowHasSurface(window)) {
      |                           ^
SDL/src/SDL_internal.h:302:34: note: expanded from macro 'CHECK_PARAM'
  302 | #define CHECK_PARAM(invalid) if (invalid)
      |                                  ^~~~~~~
SDL/src/render/SDL_render.c:1061:5: note: Taking false branch
 1061 |     CHECK_PARAM(window && SDL_WindowHasSurface(window)) {
      |     ^
SDL/src/SDL_internal.h:302:30: note: expanded from macro 'CHECK_PARAM'
  302 | #define CHECK_PARAM(invalid) if (invalid)
      |                              ^
SDL/src/render/SDL_render.c:1066:17: note: 'window' is non-null
 1066 |     CHECK_PARAM(window && SDL_GetRenderer(window)) {
      |                 ^
SDL/src/render/SDL_render.c:1066:17: note: Left side of '&&' is true
SDL/src/render/SDL_render.c:1066:27: note: Assuming the condition is false
 1066 |     CHECK_PARAM(window && SDL_GetRenderer(window)) {
      |                           ^
SDL/src/SDL_internal.h:302:34: note: expanded from macro 'CHECK_PARAM'
  302 | #define CHECK_PARAM(invalid) if (invalid)
      |                                  ^~~~~~~
SDL/src/render/SDL_render.c:1066:5: note: Taking false branch
 1066 |     CHECK_PARAM(window && SDL_GetRenderer(window)) {
      |     ^
SDL/src/SDL_internal.h:302:30: note: expanded from macro 'CHECK_PARAM'
  302 | #define CHECK_PARAM(invalid) if (invalid)
      |                              ^
SDL/src/render/SDL_render.c:1078:9: note: Assuming 'renderer' is non-null
 1078 |     if (!renderer) {
      |         ^~~~~~~~~
SDL/src/render/SDL_render.c:1078:5: note: Taking false branch
 1078 |     if (!renderer) {
      |     ^
SDL/src/render/SDL_render.c:1085:9: note: Assuming 'hint' is null
 1085 |     if (hint && *hint) {
      |         ^~~~
SDL/src/render/SDL_render.c:1085:14: note: Left side of '&&' is false
 1085 |     if (hint && *hint) {
      |              ^
SDL/src/render/SDL_render.c:1089:9: note: 'surface' is null
 1089 |     if (surface) {
      |         ^~~~~~~
SDL/src/render/SDL_render.c:1089:5: note: Taking false branch
 1089 |     if (surface) {
      |     ^
SDL/src/render/SDL_render.c:1101:13: note: Assuming 'driver_name' is non-null
 1101 |         if (!driver_name) {
      |             ^~~~~~~~~~~~
SDL/src/render/SDL_render.c:1101:9: note: Taking false branch
 1101 |         if (!driver_name) {
      |         ^
SDL/src/render/SDL_render.c:1105:13: note: 'driver_name' is non-null
 1105 |         if (driver_name && *driver_name != 0) {
      |             ^~~~~~~~~~~
SDL/src/render/SDL_render.c:1105:13: note: Left side of '&&' is true
SDL/src/render/SDL_render.c:1105:28: note: Assuming the condition is true
 1105 |         if (driver_name && *driver_name != 0) {
      |                            ^~~~~~~~~~~~~~~~~
SDL/src/render/SDL_render.c:1105:9: note: Taking true branch
 1105 |         if (driver_name && *driver_name != 0) {
      |         ^
SDL/src/render/SDL_render.c:1107:20: note: 'driver_attempt' is non-null
 1107 |             while (driver_attempt && *driver_attempt != 0 && !rc) {
      |                    ^~~~~~~~~~~~~~
SDL/src/render/SDL_render.c:1107:20: note: Left side of '&&' is true
SDL/src/render/SDL_render.c:1107:20: note: Left side of '&&' is true
SDL/src/render/SDL_render.c:1107:13: note: Loop condition is true.  Entering loop body
 1107 |             while (driver_attempt && *driver_attempt != 0 && !rc) {
      |             ^
SDL/src/render/SDL_render.c:1109:51: note: Assuming 'driver_attempt_end' is null
 1109 |                 const size_t driver_attempt_len = (driver_attempt_end) ? (driver_attempt_end - driver_attempt) : SDL_strlen(driver_attempt);
      |                                                   ^~~~~~~~~~~~~~~~~~~~
SDL/src/render/SDL_render.c:1109:51: note: '?' condition is false
SDL/src/render/SDL_render.c:1111:17: note: Loop condition is true.  Entering loop body
 1111 |                 for (int i = 0; render_drivers[i]; i++) {
      |                 ^
SDL/src/render/SDL_render.c:1113:26: note: Assuming the condition is true
 1113 |                     if ((driver_attempt_len == SDL_strlen(driver->name)) && (SDL_strncasecmp(driver->name, driver_attempt, driver_attempt_len) == 0)) {
      |                          ^~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~
SDL/src/render/SDL_render.c:1113:25: note: Left side of '&&' is true
 1113 |                     if ((driver_attempt_len == SDL_strlen(driver->name)) && (SDL_strncasecmp(driver->name, driver_attempt, driver_attempt_len) == 0)) {
      |                         ^
SDL/src/render/SDL_render.c:1113:78: note: Assuming the condition is true
 1113 |                     if ((driver_attempt_len == SDL_strlen(driver->name)) && (SDL_strncasecmp(driver->name, driver_attempt, driver_attempt_len) == 0)) {
      |                                                                              ^
SDL/include/SDL3/SDL_stdinc.h:6052:25: note: expanded from macro 'SDL_strncasecmp'
 6052 | #define SDL_strncasecmp strncasecmp
      |                         ^
SDL/src/render/SDL_render.c:1113:21: note: Taking true branch
 1113 |                     if ((driver_attempt_len == SDL_strlen(driver->name)) && (SDL_strncasecmp(driver->name, driver_attempt, driver_attempt_len) == 0)) {
      |                     ^
SDL/src/render/SDL_render.c:1116:29: note: Assuming 'rc' is false
 1116 |                         if (rc) {
      |                             ^~
SDL/src/render/SDL_render.c:1116:25: note: Taking false branch
 1116 |                         if (rc) {
      |                         ^
SDL/src/render/SDL_render.c:1119:40: note: Memory is allocated
 1119 |                         driver_error = SDL_strdup(SDL_GetError());
      |                                        ^
SDL/include/SDL3/SDL_stdinc.h:6040:20: note: expanded from macro 'SDL_strdup'
 6040 | #define SDL_strdup strdup
      |                    ^
SDL/src/render/SDL_render.c:1111:17: note: Loop condition is true.  Entering loop body
 1111 |                 for (int i = 0; render_drivers[i]; i++) {
      |                 ^
SDL/src/render/SDL_render.c:1113:26: note: Assuming the condition is true
 1113 |                     if ((driver_attempt_len == SDL_strlen(driver->name)) && (SDL_strncasecmp(driver->name, driver_attempt, driver_attempt_len) == 0)) {
      |                          ^~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~
SDL/src/render/SDL_render.c:1113:25: note: Left side of '&&' is true
 1113 |                     if ((driver_attempt_len == SDL_strlen(driver->name)) && (SDL_strncasecmp(driver->name, driver_attempt, driver_attempt_len) == 0)) {
      |                         ^
SDL/src/render/SDL_render.c:1113:21: note: Taking true branch
 1113 |                     if ((driver_attempt_len == SDL_strlen(driver->name)) && (SDL_strncasecmp(driver->name, driver_attempt, driver_attempt_len) == 0)) {
      |                     ^
SDL/src/render/SDL_render.c:1114:25: note: Memory is released
 1114 |                         SDL_free(driver_error);
      |                         ^
SDL/include/SDL3/SDL_stdinc.h:6023:18: note: expanded from macro 'SDL_free'
 6023 | #define SDL_free free
      |                  ^
SDL/src/render/SDL_render.c:1116:29: note: Assuming 'rc' is true
 1116 |                         if (rc) {
      |                             ^~
SDL/src/render/SDL_render.c:1116:25: note: Taking true branch
 1116 |                         if (rc) {
      |                         ^
SDL/src/render/SDL_render.c:1117:29: note:  Execution continues on line 1126
 1117 |                             break;
      |                             ^
SDL/src/render/SDL_render.c:1126:35: note: 'driver_attempt_end' is null
 1126 |                 driver_attempt = (driver_attempt_end) ? (driver_attempt_end + 1) : NULL;
      |                                   ^~~~~~~~~~~~~~~~~~
SDL/src/render/SDL_render.c:1126:34: note: '?' condition is false
 1126 |                 driver_attempt = (driver_attempt_end) ? (driver_attempt_end + 1) : NULL;
      |                                  ^
SDL/src/render/SDL_render.c:1107:20: note: 'driver_attempt' is null
 1107 |             while (driver_attempt && *driver_attempt != 0 && !rc) {
      |                    ^~~~~~~~~~~~~~
SDL/src/render/SDL_render.c:1107:35: note: Left side of '&&' is false
 1107 |             while (driver_attempt && *driver_attempt != 0 && !rc) {
      |                                   ^
SDL/src/render/SDL_render.c:1140:13: note: 'rc' is true
 1140 |         if (rc) {
      |             ^~
SDL/src/render/SDL_render.c:1140:9: note: Taking true branch
 1140 |         if (rc) {
      |         ^
SDL/src/render/SDL_render.c:1142:13: note: Attempt to free released memory
 1142 |             SDL_free(driver_error);
      |             ^
SDL/include/SDL3/SDL_stdinc.h:6023:18: note: expanded from macro 'SDL_free'
 6023 | #define SDL_free free
      |                  ^

driver_error is not reset, when breaking out of loop:

SDL/src/render/SDL_render.c

Lines 1114 to 1119 in 3a59163

	SDL_free(driver_error);
	rc = driver->CreateRenderer(renderer, window, props);
	if (rc) {
	break;
	}
	driver_error = SDL_strdup(SDL_GetError());

And then freed again:

SDL/src/render/SDL_render.c

Lines 1140 to 1142 in 3a59163

	if (rc) {
	SDL_DebugLogBackend("render", renderer->name);
	SDL_free(driver_error);

[slouken]

I went ahead and made it more explicit by always setting driver_error to NULL when we free it. Thanks!

[Sackzement]

Was thinking about this too, but went with the "clever" one-line fix.
Should have just went with the more readable option in the first place. :)