jingham added a comment.
Couple of minor comments.
================
Comment at: lldb/include/lldb/Target/Platform.h:855
protected:
+ lldb::ProcessSP DoConnectProcess(llvm::StringRef connect_url,
+ llvm::StringRef plugin_name,
----------------
Even though this is a private method it's still worth documenting the fact that
you are switching off of "stream != nullptr" to determine sync vrs. async
attach. That's not obvious.
================
Comment at: lldb/source/Target/Platform.cpp:1834
if (error.Fail())
return nullptr;
----------------
If you fail here you leave the process hijacked. That doesn't matter because
if "ConnectRemote" fails, you aren't going to have much to listen to anyway.
But it still looks odd. I'm surprised we don't have some RAII-dingus for
process hijacking, but anyway, it's good practice to undo this in the error
branch.
Repository:
rLLDB LLDB
CHANGES SINCE LAST ACTION
https://reviews.llvm.org/D83728/new/
https://reviews.llvm.org/D83728
_______________________________________________
lldb-commits mailing list
[email protected]
https://lists.llvm.org/cgi-bin/mailman/listinfo/lldb-commits
