Add support for SAM over UNIX domain sockets#2375
Conversation
| { | ||
| boost::asio::ip::tcp::endpoint ep; | ||
| ep.resize(tmp.size()); | ||
| std::memcpy(ep.data(), tmp.data(), tmp.size()); |
There was a problem hiding this comment.
Why do you use memcpy instead just ep = tmp?
There was a problem hiding this comment.
Why do you use memcpy instead just ep = tmp?
The SAMSocket::Socket_t is now a generic::stream_protocol::socket to allow it to receive a tcp::socket or local::stream_protocol::socket at runtime.
So remote_endpoint() now returns a generic endpoint, which could not be copied into tcp::endpoint.
There was a problem hiding this comment.
If generic_endpoint is the base class, why not use static_cast instead? Relying on internals is always a bad idea, because it can change later.
| const decltype(m_Sessions)& GetSessions () const { return m_Sessions; }; | ||
| }; | ||
|
|
||
| class SAMAcceptor |
There was a problem hiding this comment.
Better to use template ServiceAcceptor from I2PService.h implementing acceptors for different socket types.
There was a problem hiding this comment.
Better to use template ServiceAcceptor from I2PService.h implementing acceptors for different socket types.
Understood. SAMBridge needs to select the type of m_Acceptor at runtime based on the config file like I2PControl.h/cpp does it.
By using the polymorphic SAMAcceptor, SAMBridge uses only one m_Acceptor in the code instead of m_Acceptor and m_LocalAcceptor like I2PControlService does.
I could add UDSAcceptor that derives from ServiceAcceptor and adapt SAMBridge/SAMSocket accordingly. but, as a template, I will need to check if the acceptor is m_Acceptor or m_LocalAcceptor.
Or I could work on a major refactor to make ServiceAcceptor polymorphic, similar to how SAMAcceptor is now.
What do you think?
There was a problem hiding this comment.
Polymophic acceptor makes sense. I would implement it in more generic code (I2PService), because it's need in many other places beside SAM. And SOCKS proxy is the first in the list.
2 participants
closes #1751