----------------------------------------------------------- This is an automatically generated e-mail. To reply, visit: https://reviews.apache.org/r/22001/#review44225 -----------------------------------------------------------
Ship it! Looks good Till. Two thoughts: (1) We no longer need to make 'out' and 'err' be non-blocking or have the close-on-exec flag since that happens in io::redirect. In fact, see (2). (2) You can close the opened 'out' and 'err' immediately after io::redirect since they get dup'ed. Adding a comment as to why you're closing them is okay would be nice too. Keeping them open until the onAny is really okay, but just not necessary and might leave someone in the future why you did it that why versus just closing it immediately. Would you mind adding a comment next to your close-on-exec comment that suggests we should improve io::redirect to take a file path for 'to' instead of a file descriptor? Then this code would be really slim! - Benjamin Hindman On May 29, 2014, 2:14 a.m., Till Toenshoff wrote: > > ----------------------------------------------------------- > This is an automatically generated e-mail. To reply, visit: > https://reviews.apache.org/r/22001/ > ----------------------------------------------------------- > > (Updated May 29, 2014, 2:14 a.m.) > > > Review request for mesos, Benjamin Hindman and Ben Mahler. > > > Bugs: MESOS-1431 > https://issues.apache.org/jira/browse/MESOS-1431 > > > Repository: mesos-git > > > Description > ------- > > see summary. > > > Diffs > ----- > > src/slave/containerizer/mesos_containerizer.cpp d01d443 > > Diff: https://reviews.apache.org/r/22001/diff/ > > > Testing > ------- > > make check > > > Thanks, > > Till Toenshoff > >
