Merge pull request #4500 from nigredo-tori/fix-osproc-env

Fix passing environment in startProcess (win)

lib/pure/osproc.nim

@@ -400,15 +400,16 @@ when defined(Windows) and not defined(useNimRtl):
     result = cast[cstring](alloc0(res.len+1))
     copyMem(result, cstring(res), res.len)
 
-  proc buildEnv(env: StringTableRef): cstring =
+  proc buildEnv(env: StringTableRef): tuple[str: cstring, len: int] =
     var L = 0
     for key, val in pairs(env): inc(L, key.len + val.len + 2)
-    result = cast[cstring](alloc0(L+2))
+    var str = cast[cstring](alloc0(L+2))
     L = 0
     for key, val in pairs(env):
       var x = key & "=" & val
-      copyMem(addr(result[L]), cstring(x), x.len+1) # copy \0
+      copyMem(addr(str[L]), cstring(x), x.len+1) # copy \0
       inc(L, x.len+1)
+    (str, L)
 
   #proc open_osfhandle(osh: Handle, mode: int): int {.
   #  importc: "_open_osfhandle", header: "<fcntl.h>".}
@@ -526,13 +527,15 @@ when defined(Windows) and not defined(useNimRtl):
     else:
       cmdl = buildCommandLine(command, args)
     var wd: cstring = nil
-    var e: cstring = nil
+    var e = (str: nil.cstring, len: -1)
     if len(workingDir) > 0: wd = workingDir
     if env != nil: e = buildEnv(env)
     if poEchoCmd in options: echo($cmdl)
     when useWinUnicode:
       var tmp = newWideCString(cmdl)
-      var ee = newWideCString(e)
+      var ee =
+        if e.str.isNil: nil
+        else: newWideCString(e.str, e.len)
       var wwd = newWideCString(wd)
       var flags = NORMAL_PRIORITY_CLASS or CREATE_UNICODE_ENVIRONMENT
       if poDemon in options: flags = flags or CREATE_NO_WINDOW
@@ -549,7 +552,7 @@ when defined(Windows) and not defined(useNimRtl):
       if poStdErrToStdOut notin options:
         fileClose(si.hStdError)
 
-    if e != nil: dealloc(e)
+    if e.str != nil: dealloc(e.str)
     if success == 0:
       if poInteractive in result.options: close(result)
       const errInvalidParameter = 87.int

lib/system/widestrs.nim

@@ -73,11 +73,11 @@ template fastRuneAt(s: cstring, i: int, result: expr, doInc = true) =
     result = 0xFFFD
     when doInc: inc(i)
 
-iterator runes(s: cstring): int =
+iterator runes(s: cstring, L: int): int =
   var
     i = 0
     result: int
-  while s[i] != '\0':
+  while i < L:
     fastRuneAt(s, i, result, true)
     yield result
 
@@ -85,7 +85,7 @@ proc newWideCString*(source: cstring, L: int): WideCString =
   unsafeNew(result, L * 4 + 2)
   #result = cast[wideCString](alloc(L * 4 + 2))
   var d = 0
-  for ch in runes(source):
+  for ch in runes(source, L):
     if ch <=% UNI_MAX_BMP:
       if ch >=% UNI_SUR_HIGH_START and ch <=% UNI_SUR_LOW_END:
         result[d] = UNI_REPLACEMENT_CHAR

tests/osproc/passenv.nim

@@ -0,0 +1,32 @@
+discard """
+  file: "passenv.nim"
+  output: "123"
+  targets: "c c++ objc"
+"""
+
+import osproc, os, strtabs
+
+# Checks that the environment is passed correctly in startProcess
+# To do that launches a copy of itself with a new environment.
+
+if paramCount() == 0:
+  # Parent process
+
+  let env = newStringTable()
+  env["A"] = "1"
+  env["B"] = "2"
+  env["C"] = "3"
+
+  let p = startProcess(
+    getAppFilename(),
+    args = @["child"],
+    env = env,
+    options = {poStdErrToStdOut, poUsePath, poParentStreams}
+  )
+
+  discard p.waitForExit
+
+else:
+  # Child process
+  # should output "123"
+  echo getEnv("A") & getEnv("B") & getEnv("C")