----------------------------------------------------------- This is an automatically generated e-mail. To reply, visit: https://reviews.apache.org/r/58028/#review170414 -----------------------------------------------------------
I basically like this approach -I've commented on the github change: Some thoughts though: * I Think you want to get the proactor address from pn_transport_t not pn_connection_t: Transport models the low level byte stream source (which is what has the sockaddr), whereas connection models the AMQP connection stream. A disconnected connection has no address, but you can't have a disconnected transport (except maybe transiently). * You should consider an explicit sockaddr "dynamic_cast" instead of a boolean return: ```struct sockaddr* pn_proactor_addr_sockaddr(...)``` rather than ```bool pn_proactor_addr_is_sockaddr(...)```. - Andrew Stitcher On March 29, 2017, 3:11 p.m., Alan Conway wrote: > > ----------------------------------------------------------- > This is an automatically generated e-mail. To reply, visit: > https://reviews.apache.org/r/58028/ > ----------------------------------------------------------- > > (Updated March 29, 2017, 3:11 p.m.) > > > Review request for qpid, Andrew Stitcher, Cliff Jansen, and Justin Ross. > > > Repository: qpid-proton-git > > > Description > ------- > > (also on https://github.com/alanconway/qpid-proton/tree/transport-addr) > > Provides actual address information for both ends of a proactor-managed > connection using pn_proactor_addr_* functions. > > - pn_proactor_addr_* functions are clearly identified as part of proactor lib > - portable print local/remote address as string with no platform-specific > headers > - POSIX/windows can test pn_proactor_addr_is_sockaddr() to use native > sockaddr API > > This can be extended safely to non-sockaddr platforms by making > pn_proactor_addr_is_sockaddr() return false and adding > pn_proactor_addr_is_foo() > to indicate the underlying address type > > > Diffs > ----- > > proton-c/include/proton/proactor.h 974b4329c8679a105cfa2d141d48b9214be13231 > proton-c/src/proactor/libuv.c aa10f83aa6c251e869e482da5533bf771ef5a180 > proton-c/src/tests/proactor.c 80eeb9a7652e39cb5650eba35732c3df3781fbad > proton-c/src/tests/test_tools.h 0c913ff2b1c7de8d22b03faadba4b1d1b385e504 > > > Diff: https://reviews.apache.org/r/58028/diff/1/ > > > Testing > ------- > > ctest on gcc, cl, debug, release > > > Thanks, > > Alan Conway > >
