----------------------------------------------------------- This is an automatically generated e-mail. To reply, visit: https://reviews.apache.org/r/62391/#review185709 -----------------------------------------------------------
Fix it, then Ship it! 3rdparty/stout/include/stout/windows/os.hpp Lines 550 (patched) <https://reviews.apache.org/r/62391/#comment262020> So the `Process32First` iteration will actually return the 0 pid in this case? Maybe this should return `WindowsError(ERROR_INVALID_PARAMETER, ...)`? 3rdparty/stout/tests/os/process_tests.cpp Lines 186 (patched) <https://reviews.apache.org/r/62391/#comment262022> I think this would be easier to read if you retained a literal expected value: ```C #ifdef __WINDOWS__ // ... EXPECT_EQ(0, pids.get().count(init_pid)); #else EXPECT_EQ(1, pids.get().count(init_pid)); #endif ``` - James Peach On Sept. 18, 2017, 11:39 p.m., Andrew Schwartzmeyer wrote: > > ----------------------------------------------------------- > This is an automatically generated e-mail. To reply, visit: > https://reviews.apache.org/r/62391/ > ----------------------------------------------------------- > > (Updated Sept. 18, 2017, 11:39 p.m.) > > > Review request for mesos, James Peach and Joseph Wu. > > > Bugs: MESOS-7988 > https://issues.apache.org/jira/browse/MESOS-7988 > > > Repository: mesos > > > Description > ------- > > This patch fixes a bug found by running `mesos-tests` under Application > Verifier on Windows. Mesos was inadvertently attempting to get a process > handle for the System Idle Process with PID `0`, which is not permitted > by the OS. To remedy this, we now check if `os::process` receives `0` > for its argument, and return an error if so. Furthermore, we remove the > PID `0` from the `os::pids` API, as it is not useful to the programmer, > and only serves to cause errors later. Finally, the return value for > `OpenProcess` was being incorrectly checked, as the API returns a > `nullptr` on failure, not `INVALID_HANDLE_VALUE`. > > > Diffs > ----- > > 3rdparty/stout/include/stout/windows/os.hpp > f35bf312a25066a167dc66243ad9bf8333cb36a6 > 3rdparty/stout/tests/os/process_tests.cpp > 963eb4eb3fb64d627e177d760bf42b777ae0e924 > > > Diff: https://reviews.apache.org/r/62391/diff/2/ > > > Testing > ------- > > Built and ran `mesos-tests`, `libprocess-tests`, and `stout-tests` on Windows > under Application Verifier. All tests pass, and the error from `OpenProcess` > is no longer being returned. > > > Thanks, > > Andrew Schwartzmeyer > >
