Re: Review Request 64909: Removed unnecessary `SocketManager::dispose` data structure.

2018-01-12 Thread Benjamin Mahler

---
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)


Yikes, this is scary. Why did you need this guard?

It looks like we can do:

```
Option 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 
> dd4d111c4ae579420060e547dd12f8f0711c 
> 
> 
> Diff: https://reviews.apache.org/r/64909/diff/2/
> 
> 
> Testing
> ---
> 
> make check
> 
> 
> Thanks,
> 
> Benjamin Hindman
> 
>



Review Request 64909: Removed unnecessary `SocketManager::dispose` data structure.

2018-01-02 Thread Benjamin Hindman

---
This is an automatically generated e-mail. To reply, visit:
https://reviews.apache.org/r/64909/
---

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 
dd4d111c4ae579420060e547dd12f8f0711c 


Diff: https://reviews.apache.org/r/64909/diff/1/


Testing
---

make check


Thanks,

Benjamin Hindman