> On May 17, 2018, 11:21 p.m., Andrew Schwartzmeyer wrote: > > 3rdparty/stout/include/stout/internal/windows/inherit.hpp > > Lines 34 (patched) > > <https://reviews.apache.org/r/67201/diff/1/?file=2024989#file2024989line34> > > > > What do the second and third arguments represent?
Second argument is the number of attributes you want to update. The third one is reserved(windows... sigh) > On May 17, 2018, 11:21 p.m., Andrew Schwartzmeyer wrote: > > 3rdparty/stout/include/stout/internal/windows/inherit.hpp > > Lines 41-43 (patched) > > <https://reviews.apache.org/r/67201/diff/1/?file=2024989#file2024989line41> > > > > We should probably use a `std::unique_ptr<PROC_THREAD_ATTRIBUTE_LIST>` > > here for RAII. Defined an std::vector since _PROC_THREAD_ATTRIBUTE_LIST is not defined in a public header > On May 17, 2018, 11:21 p.m., Andrew Schwartzmeyer wrote: > > 3rdparty/stout/include/stout/internal/windows/inherit.hpp > > Lines 50 (patched) > > <https://reviews.apache.org/r/67201/diff/1/?file=2024989#file2024989line50> > > > > RAII takes care of this ;) > > > > I noticed below we also called `DeleteProcThreadAttributeList`. Under > > what conditions do we `DeleteProcThreadAttributeList` and `free`, and when > > do we just `free`? DeleteProcThreadAttributeList is used to release memory when proc_thread_attribute_list is initialized successfully. You have to call free to deallocate it(due to the malloc) > On May 17, 2018, 11:21 p.m., Andrew Schwartzmeyer wrote: > > 3rdparty/stout/include/stout/os/windows/shell.hpp > > Line 269 (original), 270 (patched) > > <https://reviews.apache.org/r/67201/diff/1/?file=2024990#file2024990line270> > > > > Is there a `W` version of this, or is that redundant...? I don't know. If UNICODE is defined(and it is), the function names are automatically suffixed wth W. I changed to W version to be consistent for now. > On May 17, 2018, 11:21 p.m., Andrew Schwartzmeyer wrote: > > 3rdparty/stout/include/stout/os/windows/shell.hpp > > Lines 279-291 (original), 283-306 (patched) > > <https://reviews.apache.org/r/67201/diff/1/?file=2024990#file2024990line283> > > > > (Old comment: Since the size is `constexpr` (it's always 3), I don't > > think we need to malloc on the heap. We can just use `std::array<HANDLE, > > 3>`.) > > > > Actually, I don't think you even need `handle_array` at all. `pipes` is > > already a `std::array<int_fd, 3>` (and `int_fd` in this case is `HANDLE`). > > You can use the > > [data()](http://en.cppreference.com/w/cpp/container/array/data) member of > > `std::array` to get access to the underlying `HANDLE[]`. > > Andrew Schwartzmeyer wrote: > Oops, I was rushing. `int_fd` is castable to `HANDLE` but is an > abstracted data type that has another field. So you'd need > `std::vector<HANDLE> handles(pipes->cbegin(), pipes->cend())` and then pass > `handles.data()`. But still, allocated automatically with no need to rfree :) Thanks. - Radhika ----------------------------------------------------------- This is an automatically generated e-mail. To reply, visit: https://reviews.apache.org/r/67201/#review203382 ----------------------------------------------------------- On May 17, 2018, 10:18 p.m., Radhika Jandhyala wrote: > > ----------------------------------------------------------- > This is an automatically generated e-mail. To reply, visit: > https://reviews.apache.org/r/67201/ > ----------------------------------------------------------- > > (Updated May 17, 2018, 10:18 p.m.) > > > Review request for mesos and Andrew Schwartzmeyer. > > > Bugs: MESOS-8926 > https://issues.apache.org/jira/browse/MESOS-8926 > > > Repository: mesos > > > Description > ------- > > Whitelist handles that can be inherited by a child process. > > > Diffs > ----- > > 3rdparty/stout/include/stout/internal/windows/inherit.hpp > 7dbde820e775cbaeb8db4bc4559ab432903e75ea > 3rdparty/stout/include/stout/os/windows/shell.hpp > 8da612af2888ff4d4d458ea5b16cdb08779b6f4c > > > Diff: https://reviews.apache.org/r/67201/diff/1/ > > > Testing > ------- > > > Thanks, > > Radhika Jandhyala > >
