[Lldb-commits] [PATCH] D62089: Make ConnectionFileDescription work with all sockets

[António Afonso via Phabricator via lldb-commits]

aadsm marked an inline comment as done.
aadsm added a comment.

Glad you brought the naming up, I was also not happy with it. I guess I kind of convinced myself in the end with "it's the url the socket used to create its connection". The other name I tinkered with was `GetConnectRemoteURI`, I wanted to have Connect in the name because of the naming scheme that it's being using, it seems something specifically to be used with ConnectionFileDescriptor::Connect (it's not clear to me why though). Actually now that I re read that function maybe `GetRemoteConnectionURI` would be better?



================
Comment at: lldb/unittests/Host/SocketTest.cpp:165-166
+  llvm::StringRef path;
+  EXPECT_TRUE(UriParser::Parse(socket_a_up->GetConnectURI(), scheme, hostname,
+                               port, path));
+  EXPECT_STREQ(scheme.str().c_str(), "connect");
----------------
labath wrote:
> The same issue as in the previous patch. You'll need to store the GetConnectURI result in a local variable.
Ah! thanks!


Repository:
  rG LLVM Github Monorepo

CHANGES SINCE LAST ACTION
  https://reviews.llvm.org/D62089/new/

https://reviews.llvm.org/D62089