----------------------------------------------------------- This is an automatically generated e-mail. To reply, visit: https://reviews.apache.org/r/19162/#review38361 -----------------------------------------------------------
3rdparty/libprocess/src/subprocess.cpp <https://reviews.apache.org/r/19162/#comment70457> What's not clear from reading this code is that our technique here is to reserve the first [0,size) envp entries for our allocated additional environment data, and the (size, ...] entries for the parent 'environ' data. This seems a bit confusing since I would normally expect 'size' to refer to the size of the array. Would it be simpler to just have envp contain our own allocated data (some copied from 'environ' rather than borrowed, some copied from 'environment'). This would more closely model what things will look like when we implement the TODO for os::environment. Also, you're currently placing the parent environment *after* the child environment, can you add a test that the child environment overrides what's contained in the parent environment? I'm wondering how execle deals with duplicate keys. 3rdparty/libprocess/src/subprocess.cpp <https://reviews.apache.org/r/19162/#comment70453> It looks like there is a bug here in which you're assigning environ[0] twice, right? envp[N] = environ[0]; // Loop iteration 1. envp[N+1] = environ[0]; // Loop iteration 2. This is because parentEnv begins with environ[0] (*environ) and in the first loop iteration is re-assigned to environ[0] again. - Ben Mahler On March 20, 2014, 11:40 p.m., Dominic Hamon wrote: > > ----------------------------------------------------------- > This is an automatically generated e-mail. To reply, visit: > https://reviews.apache.org/r/19162/ > ----------------------------------------------------------- > > (Updated March 20, 2014, 11:40 p.m.) > > > Review request for mesos, Benjamin Hindman and Ben Mahler. > > > Repository: mesos-git > > > Description > ------- > > See summary > > > Diffs > ----- > > 3rdparty/libprocess/include/process/subprocess.hpp > 75b2a96b3ca62a4a59e64aba046f1d797bd8ebf6 > 3rdparty/libprocess/src/subprocess.cpp PRE-CREATION > 3rdparty/libprocess/src/tests/subprocess_tests.cpp > d15d4d159105474117c4ea432b215431209ab539 > > Diff: https://reviews.apache.org/r/19162/diff/ > > > Testing > ------- > > make check > > > Thanks, > > Dominic Hamon > >
