[Lldb-commits] [PATCH] D11066: Introduce a MainLoop class and switch llgs to use it

[Oleksiy Vyalov]

ovyalov accepted this revision.
ovyalov added a comment.

Minor comments.


================
Comment at: include/lldb/Host/posix/MainLoopPosix.h:91
@@ +90,3 @@
+
+    llvm::DenseMap<int, Callback> m_read_fds;
+    llvm::DenseMap<int, SignalInfo> m_signals;
----------------
s/int/IOObject::WaitableHandle ?

================
Comment at: source/Host/posix/MainLoopPosix.cpp:108
@@ +107,3 @@
+    // We undo the actions of RegisterSignal on a best-effort basis.
+    auto it = m_signals.find(signo);
+
----------------
Could add a check for iterator != m_signals.end() ?

================
Comment at: source/Host/posix/MainLoopPosix.cpp:171
@@ +170,3 @@
+
+        for (int fd: read_fds)
+        {
----------------
Check for termination flag set here?
It seems no need to read from sockets if signal handlers requested termination?

================
Comment at: tools/lldb-server/lldb-gdbserver.cpp:411
@@ -406,3 +410,3 @@
             printf ("Connection established.\n");
-            gdb_server.SetConnection (connection_up.release());
+            gdb_server.InitializeConnection (std::move(connection_up));
         }
----------------
Could you log if error is returned?

================
Comment at: tools/lldb-server/lldb-gdbserver.cpp:470
@@ -465,3 +469,3 @@
                 printf ("Connection established '%s'\n", s_listen_connection_up->GetURI().c_str());
-                gdb_server.SetConnection (s_listen_connection_up.release());
+                gdb_server.InitializeConnection (std::move(s_listen_connection_up));
             }
----------------
ditto.


http://reviews.llvm.org/D11066