Re: Review Request 59688: Moved `net::IPNetwork` to `net::IP:Network`.

[Benjamin Hindman]

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


Ship it!




Ship It!

- Benjamin Hindman


On June 29, 2017, 11:24 p.m., Avinash sridharan wrote:
> 
> ---
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/59688/
> ---
> 
> (Updated June 29, 2017, 11:24 p.m.)
> 
> 
> Review request for mesos, Benjamin Hindman and Jie Yu.
> 
> 
> Bugs: MESOS-7488
> https://issues.apache.org/jira/browse/MESOS-7488
> 
> 
> Repository: mesos
> 
> 
> Description
> ---
> 
> Moved `net::IPNetwork` to `net::IP:Network`.
> 
> 
> Diffs
> -
> 
>   3rdparty/stout/include/stout/ip.hpp 
> ee48befac8b9bb4956aec0c5bdebd69347bd5890 
>   3rdparty/stout/tests/ip_tests.cpp 39ee0598e3580510a30894fd3f47d2014f596ef7 
> 
> 
> Diff: https://reviews.apache.org/r/59688/diff/4/
> 
> 
> Testing
> ---
> 
> make check
> 
> 
> Thanks,
> 
> Avinash sridharan
> 
>

Re: Review Request 59688: Moved `net::IPNetwork` to `net::IP:Network`.

[Avinash sridharan]

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

(Updated June 29, 2017, 11:24 p.m.)


Review request for mesos, Benjamin Hindman and Jie Yu.


Changes
---

Rebased.


Bugs: MESOS-7488
https://issues.apache.org/jira/browse/MESOS-7488


Repository: mesos


Description
---

Moved `net::IPNetwork` to `net::IP:Network`.


Diffs (updated)
-

  3rdparty/stout/include/stout/ip.hpp ee48befac8b9bb4956aec0c5bdebd69347bd5890 
  3rdparty/stout/tests/ip_tests.cpp 39ee0598e3580510a30894fd3f47d2014f596ef7 


Diff: https://reviews.apache.org/r/59688/diff/4/

Changes: https://reviews.apache.org/r/59688/diff/3-4/


Testing
---

make check


Thanks,

Avinash sridharan

Re: Review Request 59688: Moved `net::IPNetwork` to `net::IP:Network`.

[Avinash sridharan]

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

(Updated June 16, 2017, 4:50 p.m.)


Review request for mesos, Benjamin Hindman and Jie Yu.


Changes
---

Rebased.


Bugs: MESOS-7488
https://issues.apache.org/jira/browse/MESOS-7488


Repository: mesos


Description
---

Moved `net::IPNetwork` to `net::IP:Network`.


Diffs (updated)
-

  3rdparty/stout/include/stout/ip.hpp ee48befac8b9bb4956aec0c5bdebd69347bd5890 
  3rdparty/stout/tests/ip_tests.cpp 39ee0598e3580510a30894fd3f47d2014f596ef7 


Diff: https://reviews.apache.org/r/59688/diff/3/

Changes: https://reviews.apache.org/r/59688/diff/2-3/


Testing
---

make check


Thanks,

Avinash sridharan

Re: Review Request 59688: Moved `net::IPNetwork` to `net::IP:Network`.

[Avinash sridharan]

> On June 2, 2017, 12:33 a.m., Benjamin Hindman wrote:
> > 3rdparty/stout/include/stout/ip.hpp
> > Lines 208 (patched)
> > 
> >
> > Ideally we move this to net::inet::IP::Network::LOOPBACK(), but perhaps 
> > add a TODO for now if this is not trivial.

Have added a TODO for now, but will create a separate patch for 
`net::inet::IP::Network` once we have the `inet::IP` patch done.


> On June 2, 2017, 12:33 a.m., Benjamin Hindman wrote:
> > 3rdparty/stout/include/stout/ip.hpp
> > Lines 291-292 (patched)
> > 
> >
> > It would be great to have a comment here explaining why you need to use 
> > `std::unique_ptr` so others don't have to try and read your mind! ;-) I 
> > don't even always remember the C++ rules here.
> > 
> > Another thought is to use a `std::shared_ptr` here instead. The 
> > advantage being that maybe then you wouldn't need the extra copy 
> > constructor? This class is constant, i.e., you never manipulate `address_` 
> > or `netmask_`, but actually making this `std::shared_ptr` might 
> > be just as tricky. Either way, comment on why this has to be a pointer!
> 
> Avinash sridharan wrote:
> Valid point. Will add the comment.
> 
> About `std::unique_ptr` vs `std::shared_ptr`, the argument is valid (not 
> requiring the copy constructor) and I was thinking about it as well. The 
> problem I had was that semantically it seemed incorrect (in my opnion). Each 
> `IP::Network` is an individual object that does not share state with another 
> copy so thought `std::unique_ptr` better exemplifies this semantic. But I 
> also agree that this is an implementation detail and the object itself is a 
> ready only (we do not allow manipulation of the `address_` and `netmask_` 
> properties). Will defer to your judgement on the use of either.

Added the comment explaining why need smart pointers. Please re-open this if 
you still want to use `shared_ptr` instead of `unique_ptr`.


- Avinash


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


On June 2, 2017, 2:23 a.m., Avinash sridharan wrote:
> 
> ---
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/59688/
> ---
> 
> (Updated June 2, 2017, 2:23 a.m.)
> 
> 
> Review request for mesos, Benjamin Hindman and Jie Yu.
> 
> 
> Bugs: MESOS-7488
> https://issues.apache.org/jira/browse/MESOS-7488
> 
> 
> Repository: mesos
> 
> 
> Description
> ---
> 
> Moved `net::IPNetwork` to `net::IP:Network`.
> 
> 
> Diffs
> -
> 
>   3rdparty/stout/include/stout/ip.hpp 
> ad2bd922158eecd12b51b272d2a003b8ec8d3550 
>   3rdparty/stout/tests/ip_tests.cpp 4c6f81bd53413360182b447ddb149a18e406870e 
> 
> 
> Diff: https://reviews.apache.org/r/59688/diff/2/
> 
> 
> Testing
> ---
> 
> make check
> 
> 
> Thanks,
> 
> Avinash sridharan
> 
>

Re: Review Request 59688: Moved `net::IPNetwork` to `net::IP:Network`.

[Avinash sridharan]

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

(Updated June 2, 2017, 2:23 a.m.)


Review request for mesos, Benjamin Hindman and Jie Yu.


Changes
---

Addressed BenH's comments.


Summary (updated)
-

Moved `net::IPNetwork` to `net::IP:Network`.


Bugs: MESOS-7488
https://issues.apache.org/jira/browse/MESOS-7488


Repository: mesos


Description (updated)
---

Moved `net::IPNetwork` to `net::IP:Network`.


Diffs (updated)
-

  3rdparty/stout/include/stout/ip.hpp ad2bd922158eecd12b51b272d2a003b8ec8d3550 
  3rdparty/stout/tests/ip_tests.cpp 4c6f81bd53413360182b447ddb149a18e406870e 


Diff: https://reviews.apache.org/r/59688/diff/2/

Changes: https://reviews.apache.org/r/59688/diff/1-2/


Testing
---

make check


Thanks,

Avinash sridharan