----------------------------------------------------------- This is an automatically generated e-mail. To reply, visit: https://reviews.apache.org/r/64909/#review195374 -----------------------------------------------------------
Looks good other than the looping over map values. 3rdparty/libprocess/src/process.cpp Lines 1919 (patched) <https://reviews.apache.org/r/64909/#comment274549> Yikes, this is scary. Why did you need this guard? It looks like we can do: ``` Option<Address> address = addresses.get(s); CHECK_SOME(address); temps.erase(address.get()); // Might be no-op, but ok. // Or: // if (temps.contains(address.get())) { // temps.erase(address.get()); // } ``` Now that we only have the "outbound" case, `addresses` should contain `s` iff `sockets` contains s, no? - Benjamin Mahler On Jan. 3, 2018, 7:21 a.m., Benjamin Hindman wrote: > > ----------------------------------------------------------- > This is an automatically generated e-mail. To reply, visit: > https://reviews.apache.org/r/64909/ > ----------------------------------------------------------- > > (Updated Jan. 3, 2018, 7:21 a.m.) > > > Review request for mesos and Benjamin Mahler. > > > Repository: mesos > > > Description > ------- > > We used `dispose` to capture the sockets that `HttpProxy` did not want > to persist but now that `HttpProxy` does not use `SocketManager` we no > longer need to use `dispose` because `temps` is sufficient for knowing > which sockets are temporary. > > > Diffs > ----- > > 3rdparty/libprocess/src/process.cpp > 75cf1d3b6d3d257ba9bc81c68017a74a6511cebf > 3rdparty/libprocess/src/socket_manager.hpp > dd4d111c4ae579420060e547d1111d12f8f0711c > > > Diff: https://reviews.apache.org/r/64909/diff/2/ > > > Testing > ------- > > make check > > > Thanks, > > Benjamin Hindman > >
