----------------------------------------------------------- This is an automatically generated e-mail. To reply, visit: https://reviews.apache.org/r/66957/#review202583 -----------------------------------------------------------
3rdparty/stout/include/stout/os/windows/pipe.hpp Line 32 (original), 33 (patched) <https://reviews.apache.org/r/66957/#comment284435> "Overlapped pipes passed to child processes behave weirdly" 3rdparty/stout/include/stout/os/windows/pipe.hpp Lines 45 (patched) <https://reviews.apache.org/r/66957/#comment284436> Run clang-format again. There's an extra space in `; i < 10;` 3rdparty/stout/include/stout/os/windows/pipe.hpp Lines 45-58 (patched) <https://reviews.apache.org/r/66957/#comment284437> This is entirely unncessary. If we can't trust that a UUID is unique, we have other problems. This can be replaced with `std::wstring name = wide_stringify("\\.\pipe\mesos-pipe-" + id::UUID::random().toString())`. 3rdparty/stout/include/stout/os/windows/pipe.hpp Lines 74 (patched) <https://reviews.apache.org/r/66957/#comment284442> Do we need to worry about this in the remarks section? > Windows 10, version 1709: Pipes are only supported within an app-container; ie, from one UWP process to another UWP process that's part of the same app. 3rdparty/stout/include/stout/os/windows/pipe.hpp Lines 75 (patched) <https://reviews.apache.org/r/66957/#comment284439> I've been preferring `.data()` since it's a `wstring`. It just seemed more fitting, though they are the same. 3rdparty/stout/include/stout/os/windows/pipe.hpp Lines 82 (patched) <https://reviews.apache.org/r/66957/#comment284441> Maybe note that this means it's not inheritable. 3rdparty/stout/include/stout/os/windows/pipe.hpp Line 48 (original), 88-95 (patched) <https://reviews.apache.org/r/66957/#comment284445> Maybe note that we're not using `CallNamedPipe` because this is a `BYTE` pipe and not a `MESSAGE` pipe / a little expo on Windows named pipes for the uninformed reader. 3rdparty/stout/include/stout/os/windows/pipe.hpp Lines 98-100 (patched) <https://reviews.apache.org/r/66957/#comment284446> You can just create the `WindowsError` first, then `CloseHandle`, then return the error. - Andrew Schwartzmeyer On May 4, 2018, 10:20 a.m., Akash Gupta wrote: > > ----------------------------------------------------------- > This is an automatically generated e-mail. To reply, visit: > https://reviews.apache.org/r/66957/ > ----------------------------------------------------------- > > (Updated May 4, 2018, 10:20 a.m.) > > > Review request for mesos, Andrew Schwartzmeyer, Benjamin Mahler, Eric Mumau, > John Kordich, Joseph Wu, and Radhika Jandhyala. > > > Bugs: MESOS-8674 > https://issues.apache.org/jira/browse/MESOS-8674 > > > Repository: mesos > > > Description > ------- > > `os::pipe` was using `CreatePipe`, which does not support overlapped > io. Now, the implementation of `os::pipe` has been changed to use > `CreateNamedPipe`, which supports overlapped IO. The named pipe is > created with an unique random name, so it is effectively anonymous. > > > Diffs > ----- > > 3rdparty/stout/include/stout/os/windows/pipe.hpp > a3574fd6f2ff1608396b47cad8cbed88134a74ca > > > Diff: https://reviews.apache.org/r/66957/diff/1/ > > > Testing > ------- > > > Thanks, > > Akash Gupta > >
