network: address socket interface pointer instead of name#12550
network: address socket interface pointer instead of name#12550mattklein123 merged 5 commits intoenvoyproxy:masterfrom
Conversation
Signed-off-by: Florin Coras <fcoras@cisco.com>
Signed-off-by: Florin Coras <fcoras@cisco.com>
lizan
left a comment
There was a problem hiding this comment.
Thanks, this looks much better. Just some style nits.
|
|
||
| } // namespace | ||
|
|
||
| static inline const SocketInterface* sockInterfaceOrDefault(const SocketInterface* sock_interface) { |
There was a problem hiding this comment.
style nit: put this in unnamed namespace above, remove static, doesn't have to be inline?
There was a problem hiding this comment.
It's a C reflex of mine that I use instead of macros. Goal would be to convince the compiler to avoid extra function calls when not needed. But obviously, no strong opinion here ...
There was a problem hiding this comment.
C++ inline isn't guaranteed to be inlined, all are decided by compiler, it's more of working around one-definition rule.
include/envoy/network/address.h
Outdated
| * @return SocketInterface to be used with the address | ||
| */ | ||
| virtual const std::string& socketInterface() const PURE; | ||
| virtual const Network::SocketInterface* socketInterface() const PURE; |
There was a problem hiding this comment.
let's return reference, it's no longer nullable.
There was a problem hiding this comment.
We'll have to keep the const because of how we pass the SocketInterface pointer to the Address. Is that okay or were you thinking about something more complex?
There was a problem hiding this comment.
ah of couse, const wherever possible :)
Signed-off-by: Florin Coras <fcoras@cisco.com>
Signed-off-by: Florin Coras <fcoras@cisco.com>
source/common/network/address_impl.h
Outdated
|
|
||
| std::string friendly_name_; | ||
| std::string socket_interface_; | ||
| const SocketInterface* socket_interface_; |
Signed-off-by: Florin Coras <fcoras@cisco.com>
Thank you for the review! |
Signed-off-by: Florin Coras fcoras@cisco.com
Risk Level: Low
Testing: unit tests
Docs Changes: n/a
Release Notes: n/a