SDL: SDL_PROP_PROCESS_CREATE_ENVIRONMENT_POINTER is an SDL_Environment pointer

From e97f63659063ea7a35b28d9b4559bca29bbd0209 Mon Sep 17 00:00:00 2001
From: Sam Lantinga <[EMAIL REDACTED]>
Date: Sat, 14 Sep 2024 11:35:53 -0700
Subject: [PATCH] SDL_PROP_PROCESS_CREATE_ENVIRONMENT_POINTER is an
 SDL_Environment pointer

---
 include/SDL3/SDL_process.h               |  4 +---
 src/dialog/unix/SDL_zenitydialog.c       | 19 ++++++-----------
 src/misc/unix/SDL_sysurl.c               | 13 +++---------
 src/process/posix/SDL_posixprocess.c     | 23 ++++++++++----------
 src/process/windows/SDL_windowsprocess.c | 27 +++++++++++-------------
 test/testprocess.c                       | 17 ++++++---------
 6 files changed, 41 insertions(+), 62 deletions(-)

diff --git a/include/SDL3/SDL_process.h b/include/SDL3/SDL_process.h
index c7696a10df2ba..31f7bdba89fe2 100644
--- a/include/SDL3/SDL_process.h
+++ b/include/SDL3/SDL_process.h
@@ -150,9 +150,7 @@ typedef enum SDL_ProcessIO
  * - `SDL_PROP_PROCESS_CREATE_ARGS_POINTER`: an array of strings containing
  *   the program to run, any arguments, and a NULL pointer, e.g. const char
  *   *args[] = { "myprogram", "argument", NULL }. This is a required property.
- * - `SDL_PROP_PROCESS_CREATE_ENVIRONMENT_POINTER`: an array of strings
- *   containing variable=value, and a NULL pointer, e.g. const char *env[] = {
- *   "PATH=/bin:/usr/bin", NULL }. If this property is set, it will be the
+ * - `SDL_PROP_PROCESS_CREATE_ENVIRONMENT_POINTER`: an SDL_Environment pointer. If this property is set, it will be the
  *   entire environment for the process, otherwise the current environment is
  *   used.
  * - `SDL_PROP_PROCESS_CREATE_STDIN_NUMBER`: an SDL_ProcessIO value describing
diff --git a/src/dialog/unix/SDL_zenitydialog.c b/src/dialog/unix/SDL_zenitydialog.c
index 17116e39193ab..d9277421de97e 100644
--- a/src/dialog/unix/SDL_zenitydialog.c
+++ b/src/dialog/unix/SDL_zenitydialog.c
@@ -192,9 +192,8 @@ static void run_zenity(zenityArgs* arg_struct)
     SDL_DialogFileCallback callback = arg_struct->callback;
     void* userdata = arg_struct->userdata;
     SDL_Process *process = NULL;
+    char **args = NULL;
     SDL_Environment *env = NULL;
-    char **process_env = NULL;
-    char **process_args = NULL;
     int status = -1;
     size_t bytes_read = 0;
     char *container = NULL;
@@ -202,8 +201,8 @@ static void run_zenity(zenityArgs* arg_struct)
     char **array = NULL;
     bool result = false;
 
-    process_args = generate_args(arg_struct);
-    if (!process_args) {
+    args = generate_args(arg_struct);
+    if (!args) {
         goto done;
     }
 
@@ -221,14 +220,9 @@ static void run_zenity(zenityArgs* arg_struct)
     SDL_SetEnvironmentVariable(env, "ZENITY_ERROR", "2", SDL_TRUE);
     SDL_SetEnvironmentVariable(env, "ZENITY_TIMEOUT", "2", SDL_TRUE);
 
-    process_env = SDL_GetEnvironmentVariables(env);
-    if (!process_env) {
-        goto done;
-    }
-
     SDL_PropertiesID props = SDL_CreateProperties();
-    SDL_SetPointerProperty(props, SDL_PROP_PROCESS_CREATE_ARGS_POINTER, process_args);
-    SDL_SetPointerProperty(props, SDL_PROP_PROCESS_CREATE_ENVIRONMENT_POINTER, process_env);
+    SDL_SetPointerProperty(props, SDL_PROP_PROCESS_CREATE_ARGS_POINTER, args);
+    SDL_SetPointerProperty(props, SDL_PROP_PROCESS_CREATE_ENVIRONMENT_POINTER, env);
     SDL_SetNumberProperty(props, SDL_PROP_PROCESS_CREATE_STDIN_NUMBER, SDL_PROCESS_STDIO_NULL);
     SDL_SetNumberProperty(props, SDL_PROP_PROCESS_CREATE_STDOUT_NUMBER, SDL_PROCESS_STDIO_APP);
     SDL_SetNumberProperty(props, SDL_PROP_PROCESS_CREATE_STDERR_NUMBER, SDL_PROCESS_STDIO_NULL);
@@ -280,8 +274,7 @@ static void run_zenity(zenityArgs* arg_struct)
 done:
     SDL_free(array);
     SDL_free(container);
-    free_args(process_args);
-    SDL_free(process_env);
+    free_args(args);
     SDL_DestroyEnvironment(env);
     SDL_DestroyProcess(process);
 
diff --git a/src/misc/unix/SDL_sysurl.c b/src/misc/unix/SDL_sysurl.c
index e60b40981323b..1efab16699dfb 100644
--- a/src/misc/unix/SDL_sysurl.c
+++ b/src/misc/unix/SDL_sysurl.c
@@ -35,9 +35,8 @@ extern char **environ;
 
 bool SDL_SYS_OpenURL(const char *url)
 {
+    const char *args[] = { "xdg-open", url, NULL };
     SDL_Environment *env = NULL;
-    char **process_env = NULL;
-    const char *process_args[] = { "xdg-open", url, NULL };
     SDL_Process *process = NULL;
     bool result = false;
 
@@ -49,17 +48,12 @@ bool SDL_SYS_OpenURL(const char *url)
     // Clear LD_PRELOAD so Chrome opens correctly when this application is launched by Steam
     SDL_UnsetEnvironmentVariable(env, "LD_PRELOAD");
 
-    process_env = SDL_GetEnvironmentVariables(env);
-    if (!process_env) {
-        goto done;
-    }
-
     SDL_PropertiesID props = SDL_CreateProperties();
     if (!props) {
         goto done;
     }
-    SDL_SetPointerProperty(props, SDL_PROP_PROCESS_CREATE_ARGS_POINTER, process_args);
-    SDL_SetPointerProperty(props, SDL_PROP_PROCESS_CREATE_ENVIRONMENT_POINTER, process_env);
+    SDL_SetPointerProperty(props, SDL_PROP_PROCESS_CREATE_ARGS_POINTER, args);
+    SDL_SetPointerProperty(props, SDL_PROP_PROCESS_CREATE_ENVIRONMENT_POINTER, env);
     SDL_SetBooleanProperty(props, SDL_PROP_PROCESS_CREATE_BACKGROUND_BOOLEAN, true);
     process = SDL_CreateProcessWithProperties(props);
     SDL_DestroyProperties(props);
@@ -70,7 +64,6 @@ bool SDL_SYS_OpenURL(const char *url)
     result = true;
 
 done:
-    SDL_free(process_env);
     SDL_DestroyEnvironment(env);
     SDL_DestroyProcess(process);
 
diff --git a/src/process/posix/SDL_posixprocess.c b/src/process/posix/SDL_posixprocess.c
index 5331e1c01914c..23a286023598d 100644
--- a/src/process/posix/SDL_posixprocess.c
+++ b/src/process/posix/SDL_posixprocess.c
@@ -141,7 +141,8 @@ static bool AddFileDescriptorCloseActions(posix_spawn_file_actions_t *fa)
 bool SDL_SYS_CreateProcessWithProperties(SDL_Process *process, SDL_PropertiesID props)
 {
     char * const *args = SDL_GetPointerProperty(props, SDL_PROP_PROCESS_CREATE_ARGS_POINTER, NULL);
-    char * const *env = SDL_GetPointerProperty(props, SDL_PROP_PROCESS_CREATE_ENVIRONMENT_POINTER, NULL);
+    SDL_Environment *env = SDL_GetPointerProperty(props, SDL_PROP_PROCESS_CREATE_ENVIRONMENT_POINTER, SDL_GetEnvironment());
+    char **envp = NULL;
     SDL_ProcessIO stdin_option = (SDL_ProcessIO)SDL_GetNumberProperty(props, SDL_PROP_PROCESS_CREATE_STDIN_NUMBER, SDL_PROCESS_STDIO_NULL);
     SDL_ProcessIO stdout_option = (SDL_ProcessIO)SDL_GetNumberProperty(props, SDL_PROP_PROCESS_CREATE_STDOUT_NUMBER, SDL_PROCESS_STDIO_INHERITED);
     SDL_ProcessIO stderr_option = (SDL_ProcessIO)SDL_GetNumberProperty(props, SDL_PROP_PROCESS_CREATE_STDERR_NUMBER, SDL_PROCESS_STDIO_INHERITED);
@@ -151,11 +152,16 @@ bool SDL_SYS_CreateProcessWithProperties(SDL_Process *process, SDL_PropertiesID
     int stdout_pipe[2] = { -1, -1 };
     int stderr_pipe[2] = { -1, -1 };
     int fd = -1;
-    char **env_copy = NULL;
 
     // Keep the malloc() before exec() so that an OOM won't run a process at all
+    envp = SDL_GetEnvironmentVariables(env);
+    if (!envp) {
+        return false;
+    }
+
     SDL_ProcessData *data = SDL_calloc(1, sizeof(*data));
     if (!data) {
+        SDL_free(envp);
         return false;
     }
     process->internal = data;
@@ -323,11 +329,6 @@ bool SDL_SYS_CreateProcessWithProperties(SDL_Process *process, SDL_PropertiesID
         }
     }
 
-    if (!env) {
-        env_copy = SDL_GetEnvironmentVariables(SDL_GetEnvironment());
-        env = env_copy;
-    }
-
     // Spawn the new process
     if (process->background) {
         int status = -1;
@@ -340,7 +341,7 @@ bool SDL_SYS_CreateProcessWithProperties(SDL_Process *process, SDL_PropertiesID
         case 0:
             // Detach from the terminal and launch the process
             setsid();
-            if (posix_spawnp(&data->pid, args[0], &fa, &attr, args, env) != 0) {
+            if (posix_spawnp(&data->pid, args[0], &fa, &attr, args, envp) != 0) {
                 _exit(errno);
             }
             _exit(0);
@@ -357,7 +358,7 @@ bool SDL_SYS_CreateProcessWithProperties(SDL_Process *process, SDL_PropertiesID
             break;
         }
     } else {
-        if (posix_spawnp(&data->pid, args[0], &fa, &attr, args, env) != 0) {
+        if (posix_spawnp(&data->pid, args[0], &fa, &attr, args, envp) != 0) {
             SDL_SetError("posix_spawn() failed: %s", strerror(errno));
             goto posix_spawn_fail_all;
         }
@@ -387,7 +388,7 @@ bool SDL_SYS_CreateProcessWithProperties(SDL_Process *process, SDL_PropertiesID
 
     posix_spawn_file_actions_destroy(&fa);
     posix_spawnattr_destroy(&attr);
-    SDL_free(env_copy);
+    SDL_free(envp);
 
     return true;
 
@@ -418,7 +419,7 @@ bool SDL_SYS_CreateProcessWithProperties(SDL_Process *process, SDL_PropertiesID
     if (stderr_pipe[WRITE_END] >= 0) {
         close(stderr_pipe[WRITE_END]);
     }
-    SDL_free(env_copy);
+    SDL_free(envp);
     return false;
 }
 
diff --git a/src/process/windows/SDL_windowsprocess.c b/src/process/windows/SDL_windowsprocess.c
index 3820a578cb068..879883c4ca9d0 100644
--- a/src/process/windows/SDL_windowsprocess.c
+++ b/src/process/windows/SDL_windowsprocess.c
@@ -145,17 +145,12 @@ static bool join_arguments(const char * const *args, char **args_out)
     return true;
 }
 
-static bool join_env(const char * const *env, char **environment_out)
+static bool join_env(char **env, char **environment_out)
 {
     size_t len;
-    const char * const *var;
+    char **var;
     char *result;
 
-    if (!env) {
-        *environment_out = NULL;
-        return true;
-    }
-
     len = 0;
     for (var = env; *var; var++) {
         len += SDL_strlen(*var) + 1;
@@ -181,7 +176,8 @@ static bool join_env(const char * const *env, char **environment_out)
 bool SDL_SYS_CreateProcessWithProperties(SDL_Process *process, SDL_PropertiesID props)
 {
     const char * const *args = SDL_GetPointerProperty(props, SDL_PROP_PROCESS_CREATE_ARGS_POINTER, NULL);
-    const char * const *env = SDL_GetPointerProperty(props, SDL_PROP_PROCESS_CREATE_ENVIRONMENT_POINTER, NULL);
+    SDL_Environment *env = SDL_GetPointerProperty(props, SDL_PROP_PROCESS_CREATE_ENVIRONMENT_POINTER, SDL_GetEnvironment());
+    char **envp = NULL;
     SDL_ProcessIO stdin_option = (SDL_ProcessIO)SDL_GetNumberProperty(props, SDL_PROP_PROCESS_CREATE_STDIN_NUMBER, SDL_PROCESS_STDIO_NULL);
     SDL_ProcessIO stdout_option = (SDL_ProcessIO)SDL_GetNumberProperty(props, SDL_PROP_PROCESS_CREATE_STDOUT_NUMBER, SDL_PROCESS_STDIO_INHERITED);
     SDL_ProcessIO stderr_option = (SDL_ProcessIO)SDL_GetNumberProperty(props, SDL_PROP_PROCESS_CREATE_STDERR_NUMBER, SDL_PROCESS_STDIO_INHERITED);
@@ -196,12 +192,17 @@ bool SDL_SYS_CreateProcessWithProperties(SDL_Process *process, SDL_PropertiesID
     HANDLE stdin_pipe[2] = { INVALID_HANDLE_VALUE, INVALID_HANDLE_VALUE };
     HANDLE stdout_pipe[2] = { INVALID_HANDLE_VALUE, INVALID_HANDLE_VALUE };
     HANDLE stderr_pipe[2] = { INVALID_HANDLE_VALUE, INVALID_HANDLE_VALUE };
-    char **env_copy = NULL;
     bool result = false;
 
     // Keep the malloc() before exec() so that an OOM won't run a process at all
+    envp = SDL_GetEnvironmentVariables(env);
+    if (!envp) {
+        return false;
+    }
+
     SDL_ProcessData *data = SDL_calloc(1, sizeof(*data));
     if (!data) {
+        SDL_free(envp);
         return false;
     }
     process->internal = data;
@@ -210,11 +211,7 @@ bool SDL_SYS_CreateProcessWithProperties(SDL_Process *process, SDL_PropertiesID
         goto done;
     }
 
-    if (!env) {
-        env_copy = SDL_GetEnvironmentVariables(SDL_GetEnvironment());
-        env = (const char * const *)env_copy;
-    }
-    if (!join_env(env, &createprocess_env)) {
+    if (!join_env(envp, &createprocess_env)) {
         goto done;
     }
 
@@ -410,7 +407,7 @@ bool SDL_SYS_CreateProcessWithProperties(SDL_Process *process, SDL_PropertiesID
     }
     SDL_free(createprocess_cmdline);
     SDL_free(createprocess_env);
-    SDL_free(env_copy);
+    SDL_free(envp);
 
     if (!result) {
         if (stdin_pipe[WRITE_END] != INVALID_HANDLE_VALUE) {
diff --git a/test/testprocess.c b/test/testprocess.c
index 576ab0f0cb763..7fb53eeabb2cc 100644
--- a/test/testprocess.c
+++ b/test/testprocess.c
@@ -35,13 +35,12 @@ static void SDLCALL setUpProcess(void **arg) {
 
 static const char *options[] = { "/path/to/childprocess" EXE, NULL };
 
-static char **DuplicateEnvironment(const char *key0, ...)
+static SDL_Environment *DuplicateEnvironment(const char *key0, ...)
 {
     va_list ap;
     const char *keyN;
     SDL_Environment *env = SDL_GetEnvironment();
     SDL_Environment *new_env = SDL_CreateEnvironment(SDL_FALSE);
-    char **result;
 
     if (key0) {
         char *sep = SDL_strchr(key0, '=');
@@ -72,9 +71,7 @@ static char **DuplicateEnvironment(const char *key0, ...)
         va_end(ap);
     }
 
-    result = SDL_GetEnvironmentVariables(new_env);
-    SDL_DestroyEnvironment(new_env);
-    return result;
+    return new_env;
 }
 
 static int SDLCALL process_testArguments(void *arg)
@@ -216,7 +213,7 @@ static int SDLCALL process_testNewEnv(void *arg)
         "--expect-env", NULL,
         NULL,
     };
-    char **process_env;
+    SDL_Environment *process_env;
     SDL_PropertiesID props;
     SDL_Process *process = NULL;
     Sint64 pid;
@@ -235,7 +232,7 @@ static int SDLCALL process_testNewEnv(void *arg)
 
     props = SDL_CreateProperties();
     SDL_SetPointerProperty(props, SDL_PROP_PROCESS_CREATE_ARGS_POINTER, (void *)process_args);
-    SDL_SetPointerProperty(props, SDL_PROP_PROCESS_CREATE_ENVIRONMENT_POINTER, (void *)process_env);
+    SDL_SetPointerProperty(props, SDL_PROP_PROCESS_CREATE_ENVIRONMENT_POINTER, process_env);
     SDL_SetNumberProperty(props, SDL_PROP_PROCESS_CREATE_STDOUT_NUMBER, SDL_PROCESS_STDIO_APP);
     process = SDL_CreateProcessWithProperties(props);
     SDL_DestroyProperties(props);
@@ -276,15 +273,15 @@ static int SDLCALL process_testNewEnv(void *arg)
     SDLTest_AssertPass("exit_code will be != 0 when environment variable was not set");
     SDLTest_AssertCheck(exit_code == 0, "Exit code should be 0, is %d", exit_code);
     SDLTest_AssertPass("About to destroy process");
-    SDL_DestroyProcess(process);
-    SDL_free(process_env);
     SDL_free(test_env_val);
+    SDL_DestroyProcess(process);
+    SDL_DestroyEnvironment(process_env);
     return TEST_COMPLETED;
 
 failed:
     SDL_free(test_env_val);
     SDL_DestroyProcess(process);
-    SDL_free(process_env);
+    SDL_DestroyEnvironment(process_env);
     return TEST_ABORTED;
 }