[Lldb-commits] [PATCH] D14952: Modify "platform connect" to connect to processes as well

[Greg Clayton via lldb-commits]

clayborg requested changes to this revision.
clayborg added a comment.
This revision now requires changes to proceed.

So my main issue with this is the new virtual "Platform::GetPendingGdbServerList(...)" function. Can we remove this function and just put the functionality into Platform::ContectRemote()?


================
Comment at: include/lldb/Target/Platform.h:1029-1030
@@ -1021,1 +1028,4 @@
 
+        virtual size_t
+        GetPendingGdbServerList(std::vector<std::string>& connection_urls);
+
----------------
Why is something GDB server specific in Platform.h? There has to be a better way.

================
Comment at: source/Commands/CommandObjectPlatform.cpp:413-429
@@ -413,1 +412,19 @@
+                result.SetStatus (eReturnStatusSuccessFinishResult);
+
+                std::vector<std::string> connection_urls;
+                platform_sp->GetPendingGdbServerList (connection_urls);
+                for (const auto& url : connection_urls)
+                {
+                    platform_sp->ConnectProcess(url.c_str(),
+                                                nullptr,
+                                                m_interpreter.GetDebugger(),
+                                                nullptr,
+                                                error);
+                    if (error.Fail())
+                    {
+                        result.AppendError(error.AsCString());
+                        result.SetStatus (eReturnStatusFailed);
+                        break;
+                    }
+                }
             }
----------------
This should just be done inside ConnectRemote so we don't have to expose GetPendingGdbServerList() which is GDB remote specific. If we can't do this functionality inside ConnectRemote we need to add a more suitable abstract API that is protocol agnostic.

================
Comment at: source/Plugins/Platform/POSIX/PlatformPOSIX.h:191-192
@@ +190,4 @@
+
+    size_t
+    GetPendingGdbServerList(std::vector<std::string>& connection_urls) override;
+
----------------
Remove and move functionality into ConnectRemote()


http://reviews.llvm.org/D14952