----------------------------------------------------------- This is an automatically generated e-mail. To reply, visit: https://reviews.apache.org/r/41632/#review132483 -----------------------------------------------------------
src/slave/posix_signalhandler.hpp (lines 22 - 50) <https://reviews.apache.org/r/41632/#comment196684> These don't need to be indented. You can move everything back 2 spaces. src/slave/posix_signalhandler.hpp (lines 22 - 23) <https://reviews.apache.org/r/41632/#comment196701> Can we add a comment to both of these files that this is not thread safe? You've factored out some specific code, that now can be misused. src/slave/slave.cpp (line 97) <https://reviews.apache.org/r/41632/#comment196685> missing `.`. src/slave/slave.cpp (line 101) <https://reviews.apache.org/r/41632/#comment196686> missing `.`. src/slave/slave.cpp (lines 793 - 794) <https://reviews.apache.org/r/41632/#comment196692> indentation. src/slave/slave.cpp (lines 793 - 798) <https://reviews.apache.org/r/41632/#comment196694> can we use the `EXIT(EXIT_FAILURE)` codes? src/slave/slave.cpp (line 798) <https://reviews.apache.org/r/41632/#comment196693> indentation. src/slave/windows_ctrlhandler.hpp (lines 24 - 56) <https://reviews.apache.org/r/41632/#comment196695> indentation. src/slave/windows_ctrlhandler.hpp (line 43) <https://reviews.apache.org/r/41632/#comment196696> indentation. src/slave/windows_ctrlhandler.hpp (lines 46 - 47) <https://reviews.apache.org/r/41632/#comment196697> 2 blank lines. src/slave/windows_ctrlhandler.hpp (lines 51 - 54) <https://reviews.apache.org/r/41632/#comment196698> please leave a space between c-style casts: e.g. `(int) val` src/slave/windows_ctrlhandler.hpp (line 53) <https://reviews.apache.org/r/41632/#comment196702> Did you mean to set `signaledWrapper` to `NULL` here? Should we delete it first? - Joris Van Remoortere On May 2, 2016, 8:35 p.m., Daniel Pravat wrote: > > ----------------------------------------------------------- > This is an automatically generated e-mail. To reply, visit: > https://reviews.apache.org/r/41632/ > ----------------------------------------------------------- > > (Updated May 2, 2016, 8:35 p.m.) > > > Review request for mesos, Alex Naparu, Artem Harutyunyan, Alex Clemmer, Joris > Van Remoortere, and Michael Park. > > > Bugs: MESOS-3644 > https://issues.apache.org/jira/browse/MESOS-3644 > > > Repository: mesos > > > Description > ------- > > Windows: Added Console Ctrl handling in `slave.cpp`. > > > Diffs > ----- > > src/slave/posix_signalhandler.hpp PRE-CREATION > src/slave/slave.cpp d82dec2b10d496065013eb4ad6a35dc054b72553 > src/slave/windows_ctrlhandler.hpp PRE-CREATION > > Diff: https://reviews.apache.org/r/41632/diff/ > > > Testing > ------- > > OSX: make check > Windows: make > > > Thanks, > > Daniel Pravat > >
