> On April 28, 2016, 11:52 p.m., Michael Park wrote: > > 3rdparty/libprocess/src/subprocess_windows.cpp, lines 534-539 > > <https://reviews.apache.org/r/46608/diff/1/?file=1358649#file1358649line534> > > > > A few comments here. > > > > (1) I think we can simply this to: > > > > ``` > > string env; > > foreach (const string& key, const string& value, environment.get()) { > > env += key + '=' + value + '\0'; > > } > > ``` > > > > then pass `env.data()` to `createChildProcess`. > > > > (2) Currently, if the string is larger than 32767 bytes, > > `createProcessEnvironment` returns `NULL`. We subsequently pass that result > > to `createChildProcess`, which result in the following behavior: > > > > > lpEnvironment [in, optional] > > A pointer to the environment block for the new process. If this > > parameter is NULL, the new process uses the environment of the calling > > process. > > > > Is this what we want? I think we can definitely incorporate whatever > > behavior we need if we go beyond the 32767 bytes, but I'm not sure exactly > > what behavior is expected here.
We can do that. It was a bad failure mode to silently map environments that were too big to `NULL`. We should let `::CreateProcess` error out in that case which will cause us to return an `Error`. - Alex ----------------------------------------------------------- This is an automatically generated e-mail. To reply, visit: https://reviews.apache.org/r/46608/#review130974 ----------------------------------------------------------- On May 3, 2016, 5:45 a.m., Alex Clemmer wrote: > > ----------------------------------------------------------- > This is an automatically generated e-mail. To reply, visit: > https://reviews.apache.org/r/46608/ > ----------------------------------------------------------- > > (Updated May 3, 2016, 5:45 a.m.) > > > Review request for mesos, Alex Naparu, Daniel Pravat, Artem Harutyunyan, > Joris Van Remoortere, Michael Park, M Lawindi, and Yi Sun. > > > Bugs: MESOS-3637 > https://issues.apache.org/jira/browse/MESOS-3637 > > > Repository: mesos > > > Description > ------- > > Libprocess: Implemented `subprocess_windows.cpp`. > > > Diffs > ----- > > 3rdparty/libprocess/include/process/subprocess_base.hpp PRE-CREATION > 3rdparty/libprocess/include/process/windows/subprocess.hpp PRE-CREATION > 3rdparty/libprocess/src/io.cpp 83e5f04f246b46880cfc34aa56441046b569b142 > 3rdparty/libprocess/src/subprocess.cpp > bb0fcbcd0dfa455c8700247c5b4ca0473fd163c3 > 3rdparty/libprocess/src/subprocess_windows.cpp PRE-CREATION > > Diff: https://reviews.apache.org/r/46608/diff/ > > > Testing > ------- > > > Thanks, > > Alex Clemmer > >
