----------------------------------------------------------- This is an automatically generated e-mail. To reply, visit: https://reviews.apache.org/r/46608/#review131356 -----------------------------------------------------------
3rdparty/libprocess/src/subprocess_windows.cpp (line 89) <https://reviews.apache.org/r/46608/#comment195282> The initialization with `INVALID_HANDLE_VALUE` has no semantic meaning, right? We should just leave it uninitialized, since we initialize it with the `duplicateHandle` call. 3rdparty/libprocess/src/subprocess_windows.cpp (lines 102 - 106) <https://reviews.apache.org/r/46608/#comment195280> No need for `else`: ```cpp if (!result) { return WindowsError("Failed to duplicate handle of stdin file"); } return duplicate; ``` 3rdparty/libprocess/src/subprocess_windows.cpp (lines 128 - 132) <https://reviews.apache.org/r/46608/#comment195297> No need for `else`: ```cpp if (handle == INVALID_HANDLE_VALUE) { return WindowsError("Failed to get `HANDLE` for file descriptor"); } return handle; ``` 3rdparty/libprocess/src/subprocess_windows.cpp (line 190) <https://reviews.apache.org/r/46608/#comment195298> `read/only` -> `read-only` 3rdparty/libprocess/src/subprocess_windows.cpp (lines 199 - 200) <https://reviews.apache.org/r/46608/#comment195299> Is this because using `GENERIC_WRITE` with `FILE_SHARE_READ` is a no-op? 3rdparty/libprocess/src/subprocess_windows.cpp (lines 300 - 302) <https://reviews.apache.org/r/46608/#comment195302> We seem to list `in`, `out`, `err` in that order. Should do the same here. 3rdparty/libprocess/src/subprocess_windows.cpp (line 414) <https://reviews.apache.org/r/46608/#comment195307> Remove newline. 3rdparty/libprocess/src/subprocess_windows.cpp (line 568) <https://reviews.apache.org/r/46608/#comment195309> `s/thecode/the code/` 3rdparty/libprocess/src/subprocess_windows.cpp (line 583) <https://reviews.apache.org/r/46608/#comment195303> `s/stdoutfds.read/stderrfds.read/` 3rdparty/libprocess/src/subprocess_windows.cpp (line 590) <https://reviews.apache.org/r/46608/#comment195306> Why not use a `unique_ptr` or `Owned` here? - Michael Park On April 23, 2016, 11:41 p.m., Alex Clemmer wrote: > > ----------------------------------------------------------- > This is an automatically generated e-mail. To reply, visit: > https://reviews.apache.org/r/46608/ > ----------------------------------------------------------- > > (Updated April 23, 2016, 11:41 p.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/src/subprocess_windows.cpp PRE-CREATION > > Diff: https://reviews.apache.org/r/46608/diff/ > > > Testing > ------- > > > Thanks, > > Alex Clemmer > >
