From 5f773bf478b57af7cdb56a895feec610908ffdeb Mon Sep 17 00:00:00 2001 From: Dmitry Polienko Date: Mon, 18 Jul 2016 23:28:40 -0700 Subject: [PATCH 1/4] Fix passing environment in startProcess (win) Previous implementation used newWideCString, which doesn't handle strings with \0 characters. --- lib/pure/osproc.nim | 49 +++++++++++++++++++++++++++++++++++----- tests/osproc/passenv.nim | 32 ++++++++++++++++++++++++++ 2 files changed, 75 insertions(+), 6 deletions(-) create mode 100644 tests/osproc/passenv.nim diff --git a/lib/pure/osproc.nim b/lib/pure/osproc.nim index 03b77572ac..3ebc80f30e 100644 --- a/lib/pure/osproc.nim +++ b/lib/pure/osproc.nim @@ -400,7 +400,7 @@ 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 envToCString(env: StringTableRef): cstring = var L = 0 for key, val in pairs(env): inc(L, key.len + val.len + 2) result = cast[cstring](alloc0(L+2)) @@ -410,6 +410,45 @@ when defined(Windows) and not defined(useNimRtl): copyMem(addr(result[L]), cstring(x), x.len+1) # copy \0 inc(L, x.len+1) + proc envToWideCString(env: StringTableRef): WideCString = + # newWideCString stops on \0 characters, so we have to + # convert every pair separately. + const wcharSize = 2 + var rows = newSeq[tuple[str: WideCString, len: int]]() + var L = 0 # length in wide chars + + for key, val in pairs(env): + let str = newWideCString(key & "=" & val) + let row = (str: str, len: str.len) + rows.add(row) + inc(L, succ row.len) + + # Leave space for trailing \0 + result = cast[WideCString](alloc0(wcharSize * (succ L))) + + L = 0 + for row in rows: + # Copy \0 too + copyMem( + addr(result[L]), + addr(row.str[0]), + wcharSize * (succ row.len) + ) + inc(L, succ row.len) + + # Trailing \0 + result[L] = Utf16Char(0) + + proc buildEnv(env: StringTableRef): auto = + # return type is WideCString if useWinUnicode is enabled, + # otherwise cstring + if env.isNil: nil + else: + when useWinUnicode: + envToWideCString(env) + else: + envToCString(env) + #proc open_osfhandle(osh: Handle, mode: int): int {. # importc: "_open_osfhandle", header: "".} @@ -526,18 +565,16 @@ when defined(Windows) and not defined(useNimRtl): else: cmdl = buildCommandLine(command, args) var wd: cstring = nil - var e: cstring = nil + let e = buildEnv(env) 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 wwd = newWideCString(wd) var flags = NORMAL_PRIORITY_CLASS or CREATE_UNICODE_ENVIRONMENT if poDemon in options: flags = flags or CREATE_NO_WINDOW success = winlean.createProcessW(nil, tmp, nil, nil, 1, flags, - ee, wwd, si, procInfo) + e, wwd, si, procInfo) else: success = winlean.createProcessA(nil, cmdl, nil, nil, 1, NORMAL_PRIORITY_CLASS, e, wd, si, procInfo) @@ -549,7 +586,7 @@ when defined(Windows) and not defined(useNimRtl): if poStdErrToStdOut notin options: fileClose(si.hStdError) - if e != nil: dealloc(e) + if e != nil: dealloc(addr(e[0])) if success == 0: if poInteractive in result.options: close(result) const errInvalidParameter = 87.int diff --git a/tests/osproc/passenv.nim b/tests/osproc/passenv.nim new file mode 100644 index 0000000000..815f7536f5 --- /dev/null +++ b/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") From 9bd952d2c2928f2e75eacf7fca4889e0a4b39c38 Mon Sep 17 00:00:00 2001 From: Dmitry Polienko Date: Tue, 19 Jul 2016 20:33:25 -0700 Subject: [PATCH 2/4] Revert changes in osproc.nim --- lib/pure/osproc.nim | 49 ++++++--------------------------------------- 1 file changed, 6 insertions(+), 43 deletions(-) diff --git a/lib/pure/osproc.nim b/lib/pure/osproc.nim index 3ebc80f30e..03b77572ac 100644 --- a/lib/pure/osproc.nim +++ b/lib/pure/osproc.nim @@ -400,7 +400,7 @@ when defined(Windows) and not defined(useNimRtl): result = cast[cstring](alloc0(res.len+1)) copyMem(result, cstring(res), res.len) - proc envToCString(env: StringTableRef): cstring = + proc buildEnv(env: StringTableRef): cstring = var L = 0 for key, val in pairs(env): inc(L, key.len + val.len + 2) result = cast[cstring](alloc0(L+2)) @@ -410,45 +410,6 @@ when defined(Windows) and not defined(useNimRtl): copyMem(addr(result[L]), cstring(x), x.len+1) # copy \0 inc(L, x.len+1) - proc envToWideCString(env: StringTableRef): WideCString = - # newWideCString stops on \0 characters, so we have to - # convert every pair separately. - const wcharSize = 2 - var rows = newSeq[tuple[str: WideCString, len: int]]() - var L = 0 # length in wide chars - - for key, val in pairs(env): - let str = newWideCString(key & "=" & val) - let row = (str: str, len: str.len) - rows.add(row) - inc(L, succ row.len) - - # Leave space for trailing \0 - result = cast[WideCString](alloc0(wcharSize * (succ L))) - - L = 0 - for row in rows: - # Copy \0 too - copyMem( - addr(result[L]), - addr(row.str[0]), - wcharSize * (succ row.len) - ) - inc(L, succ row.len) - - # Trailing \0 - result[L] = Utf16Char(0) - - proc buildEnv(env: StringTableRef): auto = - # return type is WideCString if useWinUnicode is enabled, - # otherwise cstring - if env.isNil: nil - else: - when useWinUnicode: - envToWideCString(env) - else: - envToCString(env) - #proc open_osfhandle(osh: Handle, mode: int): int {. # importc: "_open_osfhandle", header: "".} @@ -565,16 +526,18 @@ when defined(Windows) and not defined(useNimRtl): else: cmdl = buildCommandLine(command, args) var wd: cstring = nil - let e = buildEnv(env) + var e: cstring = nil 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 wwd = newWideCString(wd) var flags = NORMAL_PRIORITY_CLASS or CREATE_UNICODE_ENVIRONMENT if poDemon in options: flags = flags or CREATE_NO_WINDOW success = winlean.createProcessW(nil, tmp, nil, nil, 1, flags, - e, wwd, si, procInfo) + ee, wwd, si, procInfo) else: success = winlean.createProcessA(nil, cmdl, nil, nil, 1, NORMAL_PRIORITY_CLASS, e, wd, si, procInfo) @@ -586,7 +549,7 @@ when defined(Windows) and not defined(useNimRtl): if poStdErrToStdOut notin options: fileClose(si.hStdError) - if e != nil: dealloc(addr(e[0])) + if e != nil: dealloc(e) if success == 0: if poInteractive in result.options: close(result) const errInvalidParameter = 87.int From c768fea6308330396e9d62aa9a6608d8fc3f40dc Mon Sep 17 00:00:00 2001 From: Dmitry Polienko Date: Tue, 19 Jul 2016 20:37:26 -0700 Subject: [PATCH 3/4] Fix newWideCString(cstring, int) --- lib/system/widestrs.nim | 6 +++--- 1 file changed, 3 insertions(+), 3 deletions(-) diff --git a/lib/system/widestrs.nim b/lib/system/widestrs.nim index 6ad0cfd589..578bebe807 100644 --- a/lib/system/widestrs.nim +++ b/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 From 46dad3cb6be73222425dea914864beefcf0b0a4a Mon Sep 17 00:00:00 2001 From: Dmitry Polienko Date: Tue, 19 Jul 2016 20:38:00 -0700 Subject: [PATCH 4/4] Fix environment handling in startProcess --- lib/pure/osproc.nim | 15 +++++++++------ 1 file changed, 9 insertions(+), 6 deletions(-) diff --git a/lib/pure/osproc.nim b/lib/pure/osproc.nim index 03b77572ac..d58335e822 100644 --- a/lib/pure/osproc.nim +++ b/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: "".} @@ -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