[Lldb-commits] [PATCH] D71105: [lldb/Reproducers] Support multiple GDB remotes

[Pavel Labath via Phabricator via lldb-commits]

labath added a comment.

Looks pretty straight-forward, but I am still not happy about how more and more of ProcessGDBRemote is infiltrating its way into the Utility module. I think that the reproducer code specific to gdb-remote should live next to the code it's capturing, and Utility/Reproducer.cpp should just provide the general architecture...



================
Comment at: lldb/include/lldb/Utility/Reproducer.h:429
 private:
+  FileSpec GetMainFile(Loader *loader) {
+    return loader->template GetFile<typename T::Info>();
----------------
is this used anywhere?


================
Comment at: lldb/source/Commands/CommandObjectReproducer.cpp:441
 
-      std::vector<GDBRemotePacket> packets;
-      yaml::Input yin((*error_or_file)->getBuffer());
-      yin >> packets;
+      static std::unique_ptr<
+          repro::MultiLoader<repro::ProcessGDBRemoteProvider>>
----------------
static? Is it not possible to dump multiple reproducer files in one session ?


================
Comment at: lldb/source/Commands/CommandObjectReproducer.cpp:447
+      llvm::Optional<std::string> gdb_file = multi_loader->GetNextFile();
+      while (gdb_file) {
+        auto error_or_file = MemoryBuffer::getFile(*gdb_file);
----------------
maybe `while ((gdb_file = multi_loader->GetNextFile()))` (with an optional `!= None`) ?


Repository:
  rLLDB LLDB

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

https://reviews.llvm.org/D71105