jasonmolenda added a comment.
Looks good. I'd recommend adding clayborg as a reviewer and giving him a
couple days to comment on this if he has time. He wrote all of this code
originally.
================
Comment at: include/lldb/Host/common/TCPSocket.h:55
+
+ std::map<int, SocketAddress> m_listen_sockets;
};
----------------
zturner wrote:
> Any particular reason you're using a `std::map` instead of a `DenseMap` or
> other similar LLVM structure? I was under the impression that the number of
> times where you should actually use `std::map` are pretty small.
llvm docs say, "Also, because DenseMap allocates space for a large number of
key/value pairs (it starts with 64 by default), it will waste a lot of space if
your keys or values are large." I think std::map is the right choice. When
it's a tossup between an llvm container class and an STL container class, my
opinion is always to go with the standard container class that everyone is
familiar with already. If there's a distinct benefit to a given llvm container
class for a specific scenario, that's fine, but it just increases the barrier
to entry for people outside the llvm community to use these classes pervasively.
================
Comment at: source/Host/common/Socket.cpp:258
regex_match.GetMatchAtIndex(host_and_port.data(), 2, port_str)) {
+ // IPv6 addresses are wrapped in [] when specified with ports
+ if (host_str.front() == '[' && host_str.back() == ']')
----------------
Ah, I'd originally said I thought we should just tack the :portnum on to the
end of the string, search backwards to find it (because the [] characters are
metacharacters in unix shells). but I see that RFC8986 says this is how things
like this are specified, like http://[2001:db8:1f70::999:de8:7648:6e8]:100/ so
cool with that.
https://reviews.llvm.org/D31823
_______________________________________________
lldb-commits mailing list
lldb-commits@lists.llvm.org
http://lists.llvm.org/cgi-bin/mailman/listinfo/lldb-commits
