labath added a comment. The patch looks fine, modulo some inline nits. I am somewhat doubtful about the name of the new function though. When I first saw `GetConnectURI`, i thought this would be the URI used to connect to `this` socket, but in reality it is the URI of the other end (right?). In that case, I think `GetRemoteURI` would be better. Or maybe, borrowing from the BSD socket vocabulary, `GetPeerURI` ? WDYT?
In D62089#1507283 <https://reviews.llvm.org/D62089#1507283>, @aadsm wrote: > I'm not really sure which source version lldb-server is being shipped with > Android Studio today (since their version works just fine obviously)... I'm not sure either, but I'd expect it's a pretty old one (>1yr) at this point. I guess this behavior regressed somehow since then.. ================ Comment at: lldb/source/Host/posix/DomainSocket.cpp:147-151 + StreamString strm; + strm.Printf("%s://%s", + GetNameOffset() == 0 ? "unix-connect" : "unix-abstract-connect", + GetSocketName().c_str()); + return strm.GetString(); ---------------- You could just write something like `llvm::formatv("{0}://{1}", GetNameOffset() == 0 ? ... , GetSocketName())` ================ 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"); ---------------- The same issue as in the previous patch. You'll need to store the GetConnectURI result in a local variable. ================ Comment at: lldb/unittests/Host/SocketTest.cpp:167 + port, path)); + EXPECT_STREQ(scheme.str().c_str(), "connect"); + EXPECT_EQ(port, socket_a_up->GetRemotePortNumber()); ---------------- `EXPECT_EQ("connect", scheme);` is enough. Repository: rG LLVM Github Monorepo CHANGES SINCE LAST ACTION https://reviews.llvm.org/D62089/new/ https://reviews.llvm.org/D62089 _______________________________________________ lldb-commits mailing list lldb-commits@lists.llvm.org https://lists.llvm.org/cgi-bin/mailman/listinfo/lldb-commits
