Replace calls to putenv with setenv (nim-lang#18530)

* Replace calls to C `putenv` with C `setenv` to remove possible memory leaks * Add test of correct behaviour on invalid input * Fix style in tests/stdlib/tos.nim Co-authored-by: Timothee Cour <[email protected]> * Update tests/stdlib/tos.nim Co-authored-by: Timothee Cour <[email protected]> * Update tests/stdlib/tos.nim Co-authored-by: Timothee Cour <[email protected]> * Add comment with bug number to tests/stdlib/tos.nim Co-authored-by: Timothee Cour <[email protected]> * Fix possible msvc arch issues Co-authored-by: Timothee Cour <[email protected]>
PMunch · Mar 28, 2022 · 75319ee · 75319ee
1 parent a561bf9
commit 75319ee
Show file tree

Hide file tree

Showing 2 changed files with 11 additions and 3 deletions.
diff --git a/lib/pure/includes/osenv.nim b/lib/pure/includes/osenv.nim
@@ -45,8 +45,10 @@ else:
 
   proc c_getenv(env: cstring): cstring {.
     importc: "getenv", header: "<stdlib.h>".}
-  proc c_putenv(env: cstring): cint {.
-    importc: "putenv", header: "<stdlib.h>".}
+  when defined(vcc):
+    proc c_putenv_s(envname: cstring, envval: cstring): cint {.importc: "_putenv_s", header: "<stdlib.h>".}
+  else:
+    proc c_setenv(envname: cstring, envval: cstring, overwrite: cint): cint {.importc: "setenv", header: "<stdlib.h>".}
   proc c_unsetenv(env: cstring): cint {.
     importc: "unsetenv", header: "<stdlib.h>".}
 
@@ -220,8 +222,11 @@ else:
           if setEnvironmentVariableW(k, v) == 0'i32: raiseOSError(osLastError())
         else:
           if setEnvironmentVariableA(key, val) == 0'i32: raiseOSError(osLastError())
+      elif defined(vcc):
+        if c_putenv_s(key, val) != 0'i32:
+          raiseOSError(osLastError())
       else:
-        if c_putenv(environment[indx]) != 0'i32:
+        if c_setenv(key, val, 1'i32) != 0'i32:
           raiseOSError(osLastError())
 
   proc delEnv*(key: string) {.tags: [WriteEnvEffect].} =

diff --git a/tests/stdlib/tos.nim b/tests/stdlib/tos.nim
@@ -614,6 +614,9 @@ block osenv:
     doAssert existsEnv(dummyEnvVar) == false
     delEnv(dummyEnvVar)         # deleting an already deleted env var
     doAssert existsEnv(dummyEnvVar) == false
+  block: # putEnv, bug #18502
+    doAssertRaises(OSError): putEnv("DUMMY_ENV_VAR_PUT=DUMMY_VALUE", "NEW_DUMMY_VALUE")
+    doAssertRaises(OSError): putEnv("", "NEW_DUMMY_VALUE")
   block:
     doAssert getEnv("DUMMY_ENV_VAR_NONEXISTENT", "") == ""
     doAssert getEnv("DUMMY_ENV_VAR_NONEXISTENT", " ") == " "