labath added a subscriber: mgorny.
labath added a comment.

This looks mostly good. The thing I'm wondering about is whether this part of 
the code is really linux specific (in which case we should rename ELFLinkMap 
into LinuxLinkMap), or if it can be useful on other platforms (in which case 
this should go into the newly-created NativeProcessELF). @mgorny , 
@krytarowski, what do you think?



================
Comment at: 
lldb/packages/Python/lldbsuite/test/tools/lldb-server/libraries-svr4/TestGdbRemoteLibrariesSvr4Support.py:21-29
+        self.test_sequence.add_log_lines(
+            [
+                # Start the inferior...
+                "read packet: $c#63"
+            ],
+            True,
+        )
----------------
This countinue-and-immediatelly-interrupt sequence seems very dodgy. What's the 
purpose of that? Given that the inferior forces a break with the null 
dereference, I would expect you don't need to send any interrupt packets here, 
just simply wait for the inferior to stop.


================
Comment at: lldb/source/Plugins/Process/Linux/NativeProcessLinux.cpp:2181
+template <typename T>
+Status NativeProcessLinux::ReadSVR4LibraryInfo(lldb::addr_t link_map_addr,
+                                               SVR4LibraryInfo &info,
----------------
labath wrote:
> This too could return an `llvm::Error`. `Expected<std::pair<SVR4LibraryInfo, 
> addr_t>>` is a bit of a mouthful, but I'd consider that instead of by-ref 
> returns too..
What about that llvm::Error?


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D62502



_______________________________________________
lldb-commits mailing list
lldb-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/lldb-commits

Reply via email to