> On Feb. 26, 2015, 6:42 p.m., Jie Yu wrote:
> > 3rdparty/libprocess/3rdparty/stout/include/stout/ip.hpp, line 122
> > <https://reviews.apache.org/r/31470/diff/1/?file=877501#file877501line122>
> >
> >     s/net mask/netmask/
> >     
> >     Here and everywhere else please.

I saw that the the change netmask -> net mask was previously done and I thought 
that this was the desired shape.
Switched back to netmask.


> On Feb. 26, 2015, 6:42 p.m., Jie Yu wrote:
> > 3rdparty/libprocess/3rdparty/stout/include/stout/ip.hpp, lines 134-135
> > <https://reviews.apache.org/r/31470/diff/1/?file=877501#file877501line134>
> >
> >     s/(in host order)//
> >     
> >     s/net mask/netmask/

Removed host order in the next patches. For this one I kept the comment because 
the IP class maintains the initial semantics. In this patch I just wanted to 
split the classes.


> On Feb. 26, 2015, 6:42 p.m., Jie Yu wrote:
> > 3rdparty/libprocess/3rdparty/stout/include/stout/ip.hpp, lines 137-139
> > <https://reviews.apache.org/r/31470/diff/1/?file=877501#file877501line137>
> >
> >     s/fromAddressNetmask/create/

Changed to create. However, in my opinion it is better to use the "from" 
pattern: fromLinkDevice, fromAddressNetmask, fromAddressPrefix.


> On Feb. 26, 2015, 6:42 p.m., Jie Yu wrote:
> > 3rdparty/libprocess/3rdparty/stout/include/stout/ip.hpp, lines 141-142
> > <https://reviews.apache.org/r/31470/diff/1/?file=877501#file877501line141>
> >
> >     Adjust the comments here. The function creates an IP network from an IP 
> > address and a subnet prefix.

I tried to be more explicit.


> On Feb. 26, 2015, 6:42 p.m., Jie Yu wrote:
> > 3rdparty/libprocess/3rdparty/stout/include/stout/ip.hpp, line 155
> > <https://reviews.apache.org/r/31470/diff/1/?file=877501#file877501line155>
> >
> >     Why return a Try here? Since we control the construction of an 
> > IPNetwork, the prefix should always be valid, right?

my mistake.


> On Feb. 26, 2015, 6:42 p.m., Jie Yu wrote:
> > 3rdparty/libprocess/3rdparty/stout/include/stout/ip.hpp, line 206
> > <https://reviews.apache.org/r/31470/diff/1/?file=877501#file877501line206>
> >
> >     The error message here can be improved. How about:
> >     
> >     ```
> >     return Error("Unexpected number of '/' detected");
> >     ```

done


> On Feb. 26, 2015, 6:42 p.m., Jie Yu wrote:
> > 3rdparty/libprocess/3rdparty/stout/include/stout/ip.hpp, lines 333-337
> > <https://reviews.apache.org/r/31470/diff/1/?file=877501#file877501line333>
> >
> >     There is a leak here, isn't it?
> >     
> >     You should move 'freeifaddrs' before the if (ip.isError()) check.

uups


> On Feb. 26, 2015, 6:42 p.m., Jie Yu wrote:
> > 3rdparty/libprocess/3rdparty/stout/tests/ip_tests.cpp, line 28
> > <https://reviews.apache.org/r/31470/diff/1/?file=877502#file877502line28>
> >
> >     s/ip/network/

done


> On Feb. 26, 2015, 6:42 p.m., Jie Yu wrote:
> > 3rdparty/libprocess/3rdparty/stout/tests/ip_tests.cpp, lines 53-54
> > <https://reviews.apache.org/r/31470/diff/1/?file=877502#file877502line53>
> >
> >     EXPECT_ERROR(net::fromLinkDevice("non-exist"));

done


> On Feb. 26, 2015, 6:42 p.m., Jie Yu wrote:
> > 3rdparty/libprocess/3rdparty/stout/tests/ip_tests.cpp, line 70
> > <https://reviews.apache.org/r/31470/diff/1/?file=877502#file877502line70>
> >
> >     s/ConstructIPNetmask/ConstructIPNetwork/

done


- Evelina


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


On Feb. 26, 2015, 6:41 a.m., Evelina Dumitrescu wrote:
> 
> -----------------------------------------------------------
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/31470/
> -----------------------------------------------------------
> 
> (Updated Feb. 26, 2015, 6:41 a.m.)
> 
> 
> Review request for mesos, Benjamin Hindman, Dominic Hamon, Jie Yu, Joris Van 
> Remoortere, and Niklas Nielsen.
> 
> 
> Repository: mesos
> 
> 
> Description
> -------
> 
> see summary
> 
> 
> Diffs
> -----
> 
>   3rdparty/libprocess/3rdparty/stout/include/stout/ip.hpp 
> 3293086a009a8f7cf7bd343eb7d3e85623636550 
>   3rdparty/libprocess/3rdparty/stout/tests/ip_tests.cpp 
> fb98317a68986cb1228c584a8cd83b07737895a8 
> 
> Diff: https://reviews.apache.org/r/31470/diff/
> 
> 
> Testing
> -------
> 
> 
> Thanks,
> 
> Evelina Dumitrescu
> 
>

Reply via email to