labath wrote: > @labath > > > Having a single socket listen on multiple ports sounds like a bad idea to > > me. > > Note that currently the class TCPSocket already contains a list of > NativeSocket `m_listen_sockets`.
I am aware of that, and I'm not entirely happy with how the class implements listening. I think it would be better to have a separate class for a listening socket, because those need a completely different APIs. But, even ignoring that, I think there's a difference between listening on two addresses with the same port (I believe the only way to reach that state is if a hostname resolves to multiple addresses, in which case one really could argue that it's the same "logical" address), and two addresses with completely unrelated ports. > We do not need 2 TCPSocket instances with 2 separated lists of native sockets > even with a common MainLoop. > > We have 2 options: > > * A) 2 threads - first one calls TCPSocket::Accept() for the platform > connection, second calls TCPSocket::Accept() for the gdb connection > > * B) 1 thread - a common TCPSocket::Accept() can accept platform and gdb > connections from both ports We have (at least) three options, the third one being what I outlined in the previous message. I'm sorry this work of yours is going to waste. I could've been more explicit about what I wanted to do on the first PR, but I was more focused on what I don't want to do. > > > I have implemented the option B. It was easy because the class TCPSocket > already contains `m_listen_sockets`. > > > Then we could create two sockets and have them listen on the same main loop > > instance. > > It is already implemented inside TCPSocket but for different addresses. That's true, but I draw a different conclusion from that. Instead of saying "this should be extended to support multiple ports", my take is that "this wasn't a good place for multiplexing to begin with". > I just added different ports. > > The changes are minimal really. Yes, but that's because you extended a stringly typed api to do that you wanted. That function is used from other places as well, and this means that other places in lldb (including user-facing code, I believe) can accept these multi-port connection strings. I don't think we want to support that. https://github.com/llvm/llvm-project/pull/104797 _______________________________________________ lldb-commits mailing list lldb-commits@lists.llvm.org https://lists.llvm.org/cgi-bin/mailman/listinfo/lldb-commits
