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



3rdparty/libprocess/3rdparty/stout/include/stout/net.hpp
<https://reviews.apache.org/r/29288/#comment114439>

    Missing .



3rdparty/libprocess/3rdparty/stout/include/stout/net.hpp
<https://reviews.apache.org/r/29288/#comment114441>

    Here and below:
    Is the initialization of the members in the constructor scope a style 
choice here for clarity? Why initialize family in the initializer list for the 
default constructor, but not for the other constructors?



3rdparty/libprocess/3rdparty/stout/include/stout/net.hpp
<https://reviews.apache.org/r/29288/#comment114444>

    32 bit unsigned integer?



3rdparty/libprocess/3rdparty/stout/include/stout/net.hpp
<https://reviews.apache.org/r/29288/#comment114452>

    Missing period.



3rdparty/libprocess/3rdparty/stout/include/stout/net.hpp
<https://reviews.apache.org/r/29288/#comment114453>

    "Unsupported family type: " + ...



3rdparty/libprocess/3rdparty/stout/include/stout/net.hpp
<https://reviews.apache.org/r/29288/#comment114454>

    same error style as above.



3rdparty/libprocess/3rdparty/stout/include/stout/net.hpp
<https://reviews.apache.org/r/29288/#comment114458>

    consistent spacing between ) and { please.



3rdparty/libprocess/3rdparty/stout/include/stout/net.hpp
<https://reviews.apache.org/r/29288/#comment114459>

    consistent spacing between 'if' and '(' please.



3rdparty/libprocess/3rdparty/stout/include/stout/net.hpp
<https://reviews.apache.org/r/29288/#comment114460>

    Missing period.



3rdparty/libprocess/3rdparty/stout/include/stout/net.hpp
<https://reviews.apache.org/r/29288/#comment114461>

    Missing Period.



3rdparty/libprocess/3rdparty/stout/include/stout/net.hpp
<https://reviews.apache.org/r/29288/#comment114464>

    I'm sorry if I missed something earlier, why is this not being passed by 
constant reference? Same for the function below.



3rdparty/libprocess/3rdparty/stout/include/stout/net.hpp
<https://reviews.apache.org/r/29288/#comment114462>

    Missing Period.



3rdparty/libprocess/3rdparty/stout/include/stout/net.hpp
<https://reviews.apache.org/r/29288/#comment114450>

    If you provide a name for this union, then everywhere that you memset / 
memcpy and take sizeof(in6Addr_), as well as everywhere that you take the 
address of it, you can replace these with the name of the union.
    
    This is more resilient to change, and removes the implicit knowledge that 
we're picking on this element in the union because we know it to be the largest 
one.


- Joris Van Remoortere


On Jan. 26, 2015, 6:56 p.m., Evelina Dumitrescu wrote:
> 
> -----------------------------------------------------------
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/29288/
> -----------------------------------------------------------
> 
> (Updated Jan. 26, 2015, 6:56 p.m.)
> 
> 
> Review request for mesos, Benjamin Hindman, Dominic Hamon, Jie Yu, Joris Van 
> Remoortere, and Niklas Nielsen.
> 
> 
> Bugs: MESOS-1919
>     https://issues.apache.org/jira/browse/MESOS-1919
> 
> 
> Repository: mesos-git
> 
> 
> Description
> -------
> 
> Created the inner class InAddrStorage encapsulated inside the IP class.
> The class uses a union with the in_addr and in6_addr fields.
> I considered that the The MasterInfo protobuffers should have both an ipv4 
> and an ipv6 field.
> I intend to use the same Classifiers, addition, removal and update of 
> container filters, but write different encode/decode functions for IPv4/ICMP 
> and IPv6/ICMPv6 because the processing of the protocol headers differ.
> 
> 
> Diffs
> -----
> 
>   3rdparty/libprocess/3rdparty/stout/include/stout/net.hpp 
> a0210ea6440086246aafe632f86498abbb70719a 
>   3rdparty/libprocess/3rdparty/stout/tests/net_tests.cpp 
> 425132e5d7c3770be4a5a39feea5a2f22179b871 
> 
> Diff: https://reviews.apache.org/r/29288/diff/
> 
> 
> Testing
> -------
> 
> make check
> 
> 
> Thanks,
> 
> Evelina Dumitrescu
> 
>

Reply via email to