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



3rdparty/libprocess/include/process/node.hpp
<https://reviews.apache.org/r/27446/#comment101529>

    this is actually a dangerous constructor as it is not marked explicit but 
can be called with a single argument. Ie, we might get an implicit cast by 
mistake.
    
    Please instead add a second constructor:
    
        Node() : ip(0), port(0) {}
        Node(uint32_t _ip, uint16_t _port)
          : ip(_ip), port(_port) {}



3rdparty/libprocess/include/process/node.hpp
<https://reviews.apache.org/r/27446/#comment101526>

    we don't need this check - it makes things more complex for a minor 
optimization.



3rdparty/libprocess/include/process/pid.hpp
<https://reviews.apache.org/r/27446/#comment101530>

    unnecessary - the default constructor for node takes care of the 0s.



3rdparty/libprocess/include/process/pid.hpp
<https://reviews.apache.org/r/27446/#comment101534>

    this can be simplified by adding an operator < to Node.



3rdparty/libprocess/src/http.cpp
<https://reviews.apache.org/r/27446/#comment101537>

    worth adding an accessor to Node to return the in_addr*?



3rdparty/libprocess/src/http.cpp
<https://reviews.apache.org/r/27446/#comment101539>

    actually, a utility method on Node to return the string host might be 
useful. Or a Try<string> host, actually.



3rdparty/libprocess/src/pid.cpp
<https://reviews.apache.org/r/27446/#comment101541>

    stream << pid.id << "@" << pid.node.host();
        
    for example :)



3rdparty/libprocess/src/process.cpp
<https://reviews.apache.org/r/27446/#comment101545>

    is this the same as
    
        if (node != __node__ && persists.count(node) == 0) {
    
    ?



3rdparty/libprocess/src/process.cpp
<https://reviews.apache.org/r/27446/#comment101546>

    define a != operator on node and define in terms of the == operator
    
        bool operator != (const Node& that) const {
          return !(*this == that);
        }


- Dominic Hamon


On Nov. 5, 2014, 7:07 p.m., Evelina Dumitrescu wrote:
> 
> -----------------------------------------------------------
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/27446/
> -----------------------------------------------------------
> 
> (Updated Nov. 5, 2014, 7:07 p.m.)
> 
> 
> Review request for mesos, Dominic Hamon, Joris Van Remoortere, and switched 
> to 'mcypark'.
> 
> 
> Bugs: MESOS-2019
>     https://issues.apache.org/jira/browse/MESOS-2019
> 
> 
> Repository: mesos-git
> 
> 
> Description
> -------
> 
> At the moment, the Node class is used to keep a mapping from a socket to the 
> ip & port pair in the process namespace.
> I want to propose to extend its use by replacing the ip & port fields from 
> the UPID class and process namespace with this type.
> 
> 
> Diffs
> -----
> 
>   3rdparty/libprocess/include/process/node.hpp 
> 7a96894456683be60595a8f96f85df37947ff5b6 
>   3rdparty/libprocess/include/process/pid.hpp 
> 23453229c2a868a62b143416de33cf630268a64e 
>   3rdparty/libprocess/include/process/process.hpp 
> 81a1f7a3fece78bb7df1e86850572922c8704ea4 
>   3rdparty/libprocess/src/http.cpp 4ef00d11de4ff4e7eb4e44aac204147799f85ac2 
>   3rdparty/libprocess/src/pid.cpp 20ff25c0959c247687955535f02e0397653af52a 
>   3rdparty/libprocess/src/process.cpp 
> 85fb9958342f0bcdde322d9c55333126e6f86668 
>   3rdparty/libprocess/src/tests/benchmarks.cpp 
> 3177a8ecac6adf8b52c3160587dc3a4fe2707d26 
>   3rdparty/libprocess/src/tests/http_tests.cpp 
> a1c3685c88d268c7ecdc2dad22608198dc0b295d 
>   3rdparty/libprocess/src/tests/metrics_tests.cpp 
> 33539e45bab8f79afa773a325465016f01fd3b75 
>   3rdparty/libprocess/src/tests/process_tests.cpp 
> b985fb77ea05fae5c0b144ea48814acc7bb5135b 
> 
> Diff: https://reviews.apache.org/r/27446/diff/
> 
> 
> Testing
> -------
> 
> make check
> 
> 
> Thanks,
> 
> Evelina Dumitrescu
> 
>

Reply via email to