[GitHub] [geode-native] pivotal-jbarrett commented on pull request #510: GEODE-7086: coding impacts for support of IPv6

2019-08-15 Thread GitHub
I think just checking that `get_addr()` is not `nullptr` should suffice. [ Full content available at: https://github.com/apache/geode-native/pull/510 ] This message was relayed via gitbox.apache.org for notifications@geode.apache.org

[GitHub] [geode-native] pivotal-jbarrett commented on pull request #510: GEODE-7086: coding impacts for support of IPv6

2019-08-15 Thread GitHub
It's an option to try. Both are better than duplicated logic. [ Full content available at: https://github.com/apache/geode-native/pull/510 ] This message was relayed via gitbox.apache.org for notifications@geode.apache.org

[GitHub] [geode-native] pivotal-jbarrett commented on pull request #510: GEODE-7086: coding impacts for support of IPv6

2019-08-15 Thread GitHub
This is where I would just put the host adder into the vector so there is no question of ownership later. [ Full content available at: https://github.com/apache/geode-native/pull/510 ] This message was relayed via gitbox.apache.org for notifications@geode.apache.org

[GitHub] [geode-native] pivotal-jbarrett commented on pull request #510: GEODE-7086: coding impacts for support of IPv6

2019-08-15 Thread GitHub
Comment not valid anymore. [ Full content available at: https://github.com/apache/geode-native/pull/510 ] This message was relayed via gitbox.apache.org for notifications@geode.apache.org

[GitHub] [geode-native] pivotal-jbarrett commented on pull request #510: GEODE-7086: coding impacts for support of IPv6

2019-08-15 Thread GitHub
Can we create a common method so we don't have duplicate IPv4 and IPv6 code in multiple places? [ Full content available at: https://github.com/apache/geode-native/pull/510 ] This message was relayed via gitbox.apache.org for notifications@geode.apache.org

[GitHub] [geode-native] pivotal-jbarrett commented on pull request #510: GEODE-7086: coding impacts for support of IPv6

2019-08-15 Thread GitHub
Perhaps add a `void writeBytes(const std::vector& bytes)` method to DataOutput? [ Full content available at: https://github.com/apache/geode-native/pull/510 ] This message was relayed via gitbox.apache.org for notifications@geode.apache.org

[GitHub] [geode-native] pivotal-jbarrett commented on pull request #510: GEODE-7086: coding impacts for support of IPv6

2019-08-15 Thread GitHub
Why not make the caller send this object the vector? Then the ambiguity of the `hostAddrLocalMem` check goes away. The owner is always the caller making the conversion to a vector. [ Full content available at: https://github.com/apache/geode-native/pull/510 ] This message was relayed via

[GitHub] [geode-native] pivotal-jbarrett commented on pull request #510: GEODE-7086: coding impacts for support of IPv6

2019-08-15 Thread GitHub
Remove commented out code. [ Full content available at: https://github.com/apache/geode-native/pull/510 ] This message was relayed via gitbox.apache.org for notifications@geode.apache.org

[GitHub] [geode-native] pivotal-jbarrett commented on pull request #510: GEODE-7086: coding impacts for support of IPv6

2019-08-15 Thread GitHub
Doesn't ACE has some encapsulation for host addr, not that I won't to increase our use of ACE but better that then our own invention right now? [ Full content available at: https://github.com/apache/geode-native/pull/510 ] This message was relayed via gitbox.apache.org for

[GitHub] [geode-native] pivotal-jbarrett commented on pull request #510: GEODE-7086: coding impacts for support of IPv6

2019-08-15 Thread GitHub
Yup, it's needed by the protocol / message here unfortunately. [ Full content available at: https://github.com/apache/geode-native/pull/510 ] This message was relayed via gitbox.apache.org for notifications@geode.apache.org