[Lldb-commits] [PATCH] D100191: [lldb] [llgs] Support owning and detaching extra processes

[Pavel Labath via Phabricator via lldb-commits]

labath added inline comments.


================
Comment at: lldb/include/lldb/Host/common/NativeProcessProtocol.h:226
+    NewSubprocess(NativeProcessProtocol *parent_process,
+                  std::unique_ptr<NativeProcessProtocol> &child_process) = 0;
   };
----------------
That way, the delegate _must_ do something with the child process.


================
Comment at: lldb/source/Plugins/Process/gdb-remote/GDBRemoteCommunicationServerLLGS.cpp:3202-3209
   if (!m_current_process ||
       (m_current_process->GetID() == LLDB_INVALID_PROCESS_ID)) {
     LLDB_LOGF(
         log,
         "GDBRemoteCommunicationServerLLGS::%s failed, no process available",
         __FUNCTION__);
     return SendErrorResponse(0x15);
----------------
I don't think this makes sense anymore....


================
Comment at: lldb/source/Plugins/Process/gdb-remote/GDBRemoteCommunicationServerLLGS.cpp:3237
+                  __FUNCTION__, it->first, error.AsCString());
+        return SendErrorResponse(0x01);
+      }
----------------
It might be better to carry on detaching even if one process fails, and then return the errors in batch (?)

Something like:
```
Error err = Error::success();
for (p : processes) {
  if (Error e = p.second.Detach().ToError())
    err = joinErrors(std::move(err), std::move(e));
}
if (err)
  SendErrorResonse(std::move(err));
```


================
Comment at: lldb/source/Plugins/Process/gdb-remote/GDBRemoteCommunicationServerLLGS.cpp:3249
 
+  if (!detached)
+    return SendErrorResponse(Status("PID %" PRIu64 " not traced", pid));
----------------
Open question: Should we return an error for a plain `D` packet, if we don't have _any_ processes around?


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

https://reviews.llvm.org/D100191