[Lldb-commits] [PATCH] D93895: Implement vAttachWait in lldb-server

[Pavel Labath via Phabricator via lldb-commits]

labath added a comment.

Thanks. This talk of all these packets has made me realize I did not completely understand the purpose of this packet. To really test the exclusion functionality of this packet, I guess we should be launching two instances of the inferior (one before sending the packet, one after), and then checking we attach to the right one. It looks like it should be easy to add to the existing test. Other than that (and some inline simplifications) this looks fine to me.



================
Comment at: lldb/source/Plugins/Process/gdb-remote/GDBRemoteCommunicationServerLLGS.cpp:360
+
+  auto is_in_exclusion_list = [&exclusion_list](ProcessInstanceInfo &info) {
+    for (auto &excluded : exclusion_list) {
----------------



================
Comment at: lldb/source/Plugins/Process/gdb-remote/GDBRemoteCommunicationServerLLGS.cpp:362-364
+      if (excluded.GetProcessID() == info.GetProcessID()) {
+        return true;
+      }
----------------
https://llvm.org/docs/CodingStandards.html#don-t-use-braces-on-simple-single-statement-bodies-of-if-else-loop-statements


================
Comment at: lldb/source/Plugins/Process/gdb-remote/GDBRemoteCommunicationServerLLGS.cpp:374-377
+      loop_process_list.erase(std::remove_if(loop_process_list.begin(),
+                                             loop_process_list.end(),
+                                             is_in_exclusion_list),
+                              loop_process_list.end());
----------------
You may need to `#include "llvm/ADT/STLExtras.h"` to get this...


================
Comment at: lldb/source/Plugins/Process/gdb-remote/GDBRemoteCommunicationServerLLGS.cpp:389-391
+        error_stream.Printf(
+            "Multiple executables with name: '%s' found. Pids: ",
+            process_name.str().c_str());
----------------



================
Comment at: lldb/source/Plugins/Process/gdb-remote/GDBRemoteCommunicationServerLLGS.cpp:393
+        for (size_t i = 0; i < loop_process_list.size() - 1; ++i) {
+          error_stream.Printf("%lu, ", loop_process_list[i].GetProcessID());
+        }
----------------



================
Comment at: lldb/source/Plugins/Process/gdb-remote/GDBRemoteCommunicationServerLLGS.cpp:395
+        }
+        error_stream.Printf("%lu.", loop_process_list.back().GetProcessID());
+
----------------



Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D93895