Browse Source

SDL_getenv() should return const

This also allows us to use SDL_FreeLater() and make SDL_getenv() thread-safe on Windows.
Sam Lantinga 9 months ago
parent
commit
29f0fd33dc

+ 1 - 1
include/SDL3/SDL_stdinc.h

@@ -662,7 +662,7 @@ extern SDL_DECLSPEC void SDLCALL SDL_aligned_free(void *mem);
  */
 extern SDL_DECLSPEC int SDLCALL SDL_GetNumAllocations(void);
 
-extern SDL_DECLSPEC char * SDLCALL SDL_getenv(const char *name);
+extern SDL_DECLSPEC const char * SDLCALL SDL_getenv(const char *name);
 extern SDL_DECLSPEC int SDLCALL SDL_setenv(const char *name, const char *value, int overwrite);
 
 typedef int (SDLCALL *SDL_CompareCallback)(const void *a, const void *b);

+ 0 - 3
src/SDL.c

@@ -49,7 +49,6 @@
 #include "joystick/SDL_joystick_c.h"
 #include "render/SDL_sysrender.h"
 #include "sensor/SDL_sensor_c.h"
-#include "stdlib/SDL_getenv_c.h"
 #include "thread/SDL_thread_c.h"
 #include "video/SDL_pixels_c.h"
 #include "video/SDL_video_c.h"
@@ -585,8 +584,6 @@ void SDL_Quit(void)
      */
     SDL_memset(SDL_SubsystemRefCount, 0x0, sizeof(SDL_SubsystemRefCount));
 
-    SDL_FreeEnvironmentMemory();
-
     SDL_QuitMainThread();
 
     SDL_bInMainQuit = SDL_FALSE;

+ 1 - 1
src/audio/SDL_wave.c

@@ -1774,7 +1774,7 @@ static int WaveLoad(SDL_IOStream *src, WaveFile *file, SDL_AudioSpec *spec, Uint
     int result;
     Uint32 chunkcount = 0;
     Uint32 chunkcountlimit = 10000;
-    char *envchunkcountlimit;
+    const char *envchunkcountlimit;
     Sint64 RIFFstart, RIFFend, lastchunkpos;
     SDL_bool RIFFlengthknown = SDL_FALSE;
     WaveFormat *format = &file->format;

+ 4 - 3
src/dynapi/SDL_dynapi.c

@@ -461,15 +461,16 @@ extern SDL_NORETURN void SDL_ExitProcess(int exitcode);
 
 static void SDL_InitDynamicAPILocked(void)
 {
-    char *libname = SDL_getenv_REAL(SDL_DYNAMIC_API_ENVVAR);
+    const char *libname = SDL_getenv_REAL(SDL_DYNAMIC_API_ENVVAR);
     SDL_DYNAPI_ENTRYFN entry = NULL; /* funcs from here by default. */
     SDL_bool use_internal = SDL_TRUE;
 
     if (libname) {
         while (*libname && !entry) {
-            char *ptr = libname;
+            // This is evil, but we're not making any permanent changes...
+            char *ptr = (char *)libname;
             while (SDL_TRUE) {
-                const char ch = *ptr;
+                char ch = *ptr;
                 if ((ch == ',') || (ch == '\0')) {
                     *ptr = '\0';
                     entry = (SDL_DYNAPI_ENTRYFN)get_sdlapi_entry(libname, "SDL_DYNAPI_entry");

+ 1 - 1
src/dynapi/SDL_dynapi_procs.h

@@ -955,7 +955,7 @@ SDL_DYNAPI_PROC(float,SDL_floorf,(float a),(a),return)
 SDL_DYNAPI_PROC(double,SDL_fmod,(double a, double b),(a,b),return)
 SDL_DYNAPI_PROC(float,SDL_fmodf,(float a, float b),(a,b),return)
 SDL_DYNAPI_PROC(void,SDL_free,(void *a),(a),)
-SDL_DYNAPI_PROC(char*,SDL_getenv,(const char *a),(a),return)
+SDL_DYNAPI_PROC(const char*,SDL_getenv,(const char *a),(a),return)
 SDL_DYNAPI_PROC(void,SDL_hid_ble_scan,(SDL_bool a),(a),)
 SDL_DYNAPI_PROC(int,SDL_hid_close,(SDL_hid_device *a),(a),return)
 SDL_DYNAPI_PROC(Uint32,SDL_hid_device_change_count,(void),(),return)

+ 8 - 5
src/filesystem/unix/SDL_sysfilesystem.c

@@ -74,18 +74,19 @@ static char *readSymLink(const char *path)
 #ifdef SDL_PLATFORM_OPENBSD
 static char *search_path_for_binary(const char *bin)
 {
-    char *envr = SDL_getenv("PATH");
+    const char *envr_real = SDL_getenv("PATH");
+    char *envr;
     size_t alloc_size;
     char *exe = NULL;
     char *start = envr;
     char *ptr;
 
-    if (!envr) {
+    if (!envr_real) {
         SDL_SetError("No $PATH set");
         return NULL;
     }
 
-    envr = SDL_strdup(envr);
+    envr = SDL_strdup(envr_real);
     if (!envr) {
         return NULL;
     }
@@ -358,7 +359,8 @@ char *SDL_SYS_GetPrefPath(const char *org, const char *app)
 static char *xdg_user_dir_lookup_with_fallback (const char *type, const char *fallback)
 {
   FILE *file;
-  char *home_dir, *config_home, *config_file;
+  const char *home_dir, *config_home;
+  char *config_file;
   char buffer[512];
   char *user_dir;
   char *p, *d;
@@ -486,7 +488,8 @@ error2:
 
 static char *xdg_user_dir_lookup (const char *type)
 {
-    char *dir, *home_dir, *user_dir;
+    const char *home_dir;
+    char *dir, *user_dir;
 
     dir = xdg_user_dir_lookup_with_fallback(type, NULL);
     if (dir)

+ 55 - 63
src/stdlib/SDL_getenv.c

@@ -30,28 +30,19 @@
 #include "../core/android/SDL_android.h"
 #endif
 
-#if (defined(SDL_PLATFORM_WIN32) || defined(SDL_PLATFORM_WINGDK)) && (!defined(HAVE_SETENV) || !defined(HAVE_GETENV))
-/* Note this isn't thread-safe! */
-static char *SDL_envmem = NULL;
-static size_t SDL_envmemlen = 0;
-
-void SDL_FreeEnvironmentMemory(void)
-{
-    if (SDL_envmem) {
-        SDL_free(SDL_envmem);
-        SDL_envmem = NULL;
-        SDL_envmemlen = 0;
-    }
-}
+#if (defined(HAVE_GETENV) && defined(HAVE_SETENV)) || \
+    (defined(HAVE_GETENV) && defined(HAVE_PUTENV) && defined(HAVE_UNSETENV))
+#define HAVE_LIBC_ENVIRONMENT
+#elif defined(SDL_PLATFORM_WIN32) || defined(SDL_PLATFORM_WINGDK)
+#define HAVE_WIN32_ENVIRONMENT
 #else
-void SDL_FreeEnvironmentMemory(void)
-{
-}
+#define HAVE_LOCAL_ENVIRONMENT
 #endif
 
 /* Put a variable into the environment */
 /* Note: Name may not contain a '=' character. (Reference: http://www.unix.com/man-page/Linux/3/setenv/) */
-#ifdef HAVE_SETENV
+#ifdef HAVE_LIBC_ENVIRONMENT
+#if defined(HAVE_SETENV)
 int SDL_setenv(const char *name, const char *value, int overwrite)
 {
     /* Input validation */
@@ -61,29 +52,10 @@ int SDL_setenv(const char *name, const char *value, int overwrite)
 
     return setenv(name, value, overwrite);
 }
-#elif defined(SDL_PLATFORM_WIN32) || defined(SDL_PLATFORM_WINGDK)
-int SDL_setenv(const char *name, const char *value, int overwrite)
-{
-    /* Input validation */
-    if (!name || *name == '\0' || SDL_strchr(name, '=') != NULL || !value) {
-        return -1;
-    }
-
-    if (!overwrite) {
-        if (GetEnvironmentVariableA(name, NULL, 0) > 0) {
-            return 0; /* asked not to overwrite existing value. */
-        }
-    }
-    if (!SetEnvironmentVariableA(name, *value ? value : NULL)) {
-        return -1;
-    }
-    return 0;
-}
 /* We have a real environment table, but no real setenv? Fake it w/ putenv. */
-#elif (defined(HAVE_GETENV) && defined(HAVE_PUTENV) && defined(HAVE_UNSETENV) && !defined(HAVE_SETENV))
+#else
 int SDL_setenv(const char *name, const char *value, int overwrite)
 {
-    size_t len;
     char *new_variable;
 
     /* Input validation */
@@ -100,17 +72,36 @@ int SDL_setenv(const char *name, const char *value, int overwrite)
     }
 
     /* This leaks. Sorry. Get a better OS so we don't have to do this. */
-    len = SDL_strlen(name) + SDL_strlen(value) + 2;
-    new_variable = (char *)SDL_malloc(len);
+    SDL_aprintf(&new_variable, "%s=%s", name, value);
     if (!new_variable) {
         return -1;
     }
-
-    SDL_snprintf(new_variable, len, "%s=%s", name, value);
     return putenv(new_variable);
 }
+#endif
+#elif defined(HAVE_WIN32_ENVIRONMENT)
+int SDL_setenv(const char *name, const char *value, int overwrite)
+{
+    /* Input validation */
+    if (!name || *name == '\0' || SDL_strchr(name, '=') != NULL || !value) {
+        return -1;
+    }
+
+    if (!overwrite) {
+        if (GetEnvironmentVariableA(name, NULL, 0) > 0) {
+            return 0; /* asked not to overwrite existing value. */
+        }
+    }
+    if (!SetEnvironmentVariableA(name, *value ? value : NULL)) {
+        return -1;
+    }
+    return 0;
+}
 #else /* roll our own */
-static char **SDL_env = (char **)0;
+
+/* We'll leak this, as environment variables are intended to persist past SDL_Quit() */
+static char **SDL_env;
+
 int SDL_setenv(const char *name, const char *value, int overwrite)
 {
     int added;
@@ -172,11 +163,11 @@ int SDL_setenv(const char *name, const char *value, int overwrite)
     }
     return added ? 0 : -1;
 }
-#endif
+#endif // HAVE_LIBC_ENVIRONMENT
 
 /* Retrieve a variable named "name" from the environment */
-#ifdef HAVE_GETENV
-char *SDL_getenv(const char *name)
+#ifdef HAVE_LIBC_ENVIRONMENT
+const char *SDL_getenv(const char *name)
 {
 #ifdef SDL_PLATFORM_ANDROID
     /* Make sure variables from the application manifest are available */
@@ -190,34 +181,35 @@ char *SDL_getenv(const char *name)
 
     return getenv(name);
 }
-#elif defined(SDL_PLATFORM_WIN32) || defined(SDL_PLATFORM_WINGDK)
-char *SDL_getenv(const char *name)
+#elif defined(HAVE_WIN32_ENVIRONMENT)
+const char *SDL_getenv(const char *name)
 {
-    size_t bufferlen;
+    DWORD length, maxlen = 0;
+    char *retval = NULL;
 
     /* Input validation */
     if (!name || *name == '\0') {
         return NULL;
     }
 
-    bufferlen =
-        GetEnvironmentVariableA(name, SDL_envmem, (DWORD)SDL_envmemlen);
-    if (bufferlen == 0) {
-        return NULL;
-    }
-    if (bufferlen > SDL_envmemlen) {
-        char *newmem = (char *)SDL_realloc(SDL_envmem, bufferlen);
-        if (!newmem) {
-            return NULL;
+    for ( ; ; ) {
+        length = GetEnvironmentVariableA(name, retval, maxlen);
+
+        if (length > maxlen) {
+            char *string = (char *)SDL_realloc(retval, length);
+            if (!string)  {
+                return NULL;
+            }
+            retval = string;
+            maxlen = length;
+        } else {
+            break;
         }
-        SDL_envmem = newmem;
-        SDL_envmemlen = bufferlen;
-        GetEnvironmentVariableA(name, SDL_envmem, (DWORD)SDL_envmemlen);
     }
-    return SDL_envmem;
+    return SDL_FreeLater(retval);
 }
 #else
-char *SDL_getenv(const char *name)
+const char *SDL_getenv(const char *name)
 {
     size_t len, i;
     char *value;
@@ -239,4 +231,4 @@ char *SDL_getenv(const char *name)
     }
     return value;
 }
-#endif
+#endif // HAVE_LIBC_ENVIRONMENT

+ 1 - 1
src/video/SDL_blit_N.c

@@ -878,7 +878,7 @@ static enum blit_features GetBlitFeatures(void)
     static enum blit_features features = -1;
     if (features == (enum blit_features) - 1) {
         /* Provide an override for testing .. */
-        char *override = SDL_getenv("SDL_ALTIVEC_BLIT_FEATURES");
+        const char *override = SDL_getenv("SDL_ALTIVEC_BLIT_FEATURES");
         if (override) {
             unsigned int features_as_uint = 0;
             SDL_sscanf(override, "%u", &features_as_uint);

+ 2 - 2
src/video/vita/SDL_vitagl_pvr.c

@@ -46,8 +46,8 @@ static void getFBSize(int *width, int *height)
 int VITA_GL_LoadLibrary(SDL_VideoDevice *_this, const char *path)
 {
     PVRSRV_PSP2_APPHINT hint;
-    char *override = SDL_getenv("VITA_MODULE_PATH");
-    char *skip_init = SDL_getenv("VITA_PVR_SKIP_INIT");
+    const char *override = SDL_getenv("VITA_MODULE_PATH");
+    const char *skip_init = SDL_getenv("VITA_PVR_SKIP_INIT");
     char *default_path = "app0:module";
     char target_path[MAX_PATH];
 

+ 3 - 3
src/video/vita/SDL_vitagles_pvr.c

@@ -35,9 +35,9 @@
 int VITA_GLES_LoadLibrary(SDL_VideoDevice *_this, const char *path)
 {
     PVRSRV_PSP2_APPHINT hint;
-    char *override = SDL_getenv("VITA_MODULE_PATH");
-    char *skip_init = SDL_getenv("VITA_PVR_SKIP_INIT");
-    char *default_path = "app0:module";
+    const char *override = SDL_getenv("VITA_MODULE_PATH");
+    const char *skip_init = SDL_getenv("VITA_PVR_SKIP_INIT");
+    const char *default_path = "app0:module";
     char target_path[MAX_PATH];
 
     if (!skip_init) { // we don't care about actual value

+ 2 - 2
src/video/vita/SDL_vitatouch.c

@@ -41,8 +41,8 @@ struct
     float range;
 } force_info[SCE_TOUCH_PORT_MAX_NUM];
 
-char *disableFrontPoll = NULL;
-char *disableBackPoll = NULL;
+const char *disableFrontPoll = NULL;
+const char *disableBackPoll = NULL;
 
 void VITA_InitTouch(void)
 {

+ 1 - 1
src/video/vita/SDL_vitavideo.c

@@ -170,7 +170,7 @@ int VITA_VideoInit(SDL_VideoDevice *_this)
 {
     SDL_DisplayMode mode;
 #ifdef SDL_VIDEO_VITA_PVR
-    char *res = SDL_getenv("VITA_RESOLUTION");
+    const char *res = SDL_getenv("VITA_RESOLUTION");
 #endif
     SDL_zero(mode);
 

+ 1 - 1
src/video/wayland/SDL_waylandshmbuffer.c

@@ -77,7 +77,7 @@ static int CreateTempFD(off_t size)
 #endif
     {
         static const char template[] = "/sdl-shared-XXXXXX";
-        char *xdg_path;
+        const char *xdg_path;
         char tmp_path[PATH_MAX];
 
         xdg_path = SDL_getenv("XDG_RUNTIME_DIR");

+ 1 - 1
test/testautomation_stdlib.c

@@ -546,7 +546,7 @@ static int stdlib_getsetenv(void *arg)
     char *value2;
     char *expected;
     int overwrite;
-    char *text;
+    const char *text;
 
     /* Create a random name. This tests SDL_getenv, since we need to */
     /* make sure the variable is not set yet (it shouldn't). */