mesos-reviews mailing list archives

Site index · List index
Message view « Date » · « Thread »
Top « Date » · « Thread »
From Benjamin Hindman <b...@berkeley.edu>
Subject Re: Review Request 59129: Introduced `inet6::Address` to handle IPv6 addresses in `libprocess`.
Date Fri, 26 May 2017 00:00:46 GMT

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




3rdparty/libprocess/include/process/address.hpp
Line 152 (original), 164 (patched)
<https://reviews.apache.org/r/59129/#comment249495>

    s/unspec/internal/



3rdparty/libprocess/include/process/address.hpp
Lines 152-154 (original), 164-166 (patched)
<https://reviews.apache.org/r/59129/#comment249502>

    Let's leave a comment here explaining that we need an internal Address to capture the
common functions reused between both inet::Address and inet6::Address which are both IP based
and that we can't do this for the top-level Address because it can potentially hold a unix
address which we can't have these overloaded functions apply to.



3rdparty/libprocess/include/process/address.hpp
Lines 207 (patched)
<https://reviews.apache.org/r/59129/#comment249496>

    s/unspec/internal/



3rdparty/libprocess/include/process/address.hpp
Line 163 (original), 221 (patched)
<https://reviews.apache.org/r/59129/#comment249498>

    Move static's up to the top of class (THIS WAS MY BUG NOT YOURS SORRY!).



3rdparty/libprocess/include/process/address.hpp
Lines 223 (patched)
<https://reviews.apache.org/r/59129/#comment249499>

    Keep using IP/port constructor after net::IP refactor.



3rdparty/libprocess/include/process/address.hpp
Line 199 (original), 281 (patched)
<https://reviews.apache.org/r/59129/#comment249500>

    Move '{' to next line.



3rdparty/libprocess/include/process/address.hpp
Line 200 (original), 282 (patched)
<https://reviews.apache.org/r/59129/#comment249501>

    Call IP/port constructor after net::IP refactor.



3rdparty/libprocess/include/process/address.hpp
Lines 285 (patched)
<https://reviews.apache.org/r/59129/#comment249503>

    inet6::Address::ANY_ANY() is explicit enough, so let's kill the extra '6'.



3rdparty/libprocess/include/process/address.hpp
Line 253 (original), 358 (patched)
<https://reviews.apache.org/r/59129/#comment249506>

    Weird spacing, probably not your bug but can you fix it!



3rdparty/libprocess/include/process/address.hpp
Line 258 (original), 363 (patched)
<https://reviews.apache.org/r/59129/#comment249505>

    Extra newline.



3rdparty/libprocess/include/process/address.hpp
Lines 377 (patched)
<https://reviews.apache.org/r/59129/#comment249504>

    Extra newline.



3rdparty/libprocess/include/process/address.hpp
Lines 387 (patched)
<https://reviews.apache.org/r/59129/#comment249507>

    Pull '{' back to previous line, below too please.



3rdparty/libprocess/include/process/address.hpp
Lines 402 (patched)
<https://reviews.apache.org/r/59129/#comment249509>

    s/unspec/internal/



3rdparty/libprocess/include/process/address.hpp
Line 271 (original), 413 (patched)
<https://reviews.apache.org/r/59129/#comment249508>

    Compiler won't catch this so just mentioning it for you.


- Benjamin Hindman


On May 10, 2017, 7:07 a.m., Avinash sridharan wrote:
> 
> -----------------------------------------------------------
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/59129/
> -----------------------------------------------------------
> 
> (Updated May 10, 2017, 7:07 a.m.)
> 
> 
> Review request for mesos and Jie Yu.
> 
> 
> Bugs: MESOS-7488
>     https://issues.apache.org/jira/browse/MESOS-7488
> 
> 
> Repository: mesos
> 
> 
> Description
> -------
> 
> Introduced `inet6::Address` to handle IPv6 addresses in `libprocess`.
> 
> 
> Diffs
> -----
> 
>   3rdparty/libprocess/include/process/address.hpp 6b143c3d00c1d7ebd1697c26b6d312a64f30839a

>   3rdparty/libprocess/src/socket.cpp 85920a1abb3a2b2da167647dcf1351b825c72c80 
> 
> 
> Diff: https://reviews.apache.org/r/59129/diff/1/
> 
> 
> Testing
> -------
> 
> make check.
> 
> 
> Thanks,
> 
> Avinash sridharan
> 
>


Mime
  • Unnamed multipart/alternative (inline, None, 0 bytes)
View raw message