[Lldb-commits] [PATCH] Reduce number of threads in lldb-mi.

Ilia K ki.stfu at gmail.com
Thu Feb 19 10:03:54 PST 2015


Looks good. What about further cleaning? We can clean CMICmnStreamStdin and remove MICmnStreamStdinLinux MICmnStreamStdinWindows classes. You want to do it in separate review?


================
Comment at: tools/lldb-mi/MICmnStreamStdin.h:125-126
@@ -124,2 +124,4 @@
     IOSStdinHandler *m_pStdinReadHandler;
+    static const int m_constBufferSize = 2048;
+    char m_cmdBuffer[m_constBufferSize];
 };
----------------
Let's do it the same way as in MICmnStreamStdinLinux.cpp:

* Specify m_constBufferSize and m_pCmdBuffer in header file:
```
 67     // Attributes:
 68   private:
 69     const MIuint m_constBufferSize;
 70     FILE *m_pStdin;
 71     MIchar *m_pCmdBuffer;
```

* Initialize in ctor:
```
 42 CMICmnStreamStdinLinux::CMICmnStreamStdinLinux(void)
 43     : m_constBufferSize(1024)
 44     , m_pStdin(nullptr)
 45     , m_pCmdBuffer(nullptr)
 46     , m_waitForInput(true)
```

* Create buffer in CMICmnStreamStdinLinux::Initialize:
```
 83     // Other resources required
 84     if (bOk)
 85     {
 86         m_pCmdBuffer = new MIchar[m_constBufferSize];
 87         m_pStdin = stdin;
 88     }
```

* Use it in fgets:
```
199     // Read user input
200     const MIchar *pText = ::fgets(&m_pCmdBuffer[0], m_constBufferSize, stdin);
```

* Don't forget to free it in CMICmnStreamStdinLinux::Shutdown:
```
126     // Tidy up
127     if (m_pCmdBuffer != nullptr)
128     {
129         delete[] m_pCmdBuffer;
130         m_pCmdBuffer = nullptr;
131     }
```

================
Comment at: tools/lldb-mi/MIDriver.cpp:968
@@ -1095,3 +967,3 @@
     {
-        InjectMICommand("-exec-interrupt");
+        InterpretCommand("-exec-interrupt");
         return;
----------------
The InjectMICommand prints the passed command using m_rStdOut.WriteMIResponse. You should do the same.

================
Comment at: tools/lldb-mi/MIDriver.cpp:1177-1179
@@ -1304,5 +1176,5 @@
 {
     const CMIUtilString strCmd(CMIUtilString::Format("-file-exec-and-symbols %s", m_strCmdLineArgExecuteableFileNamePath.c_str()));
 
-    return InjectMICommand(strCmd);
+    return InterpretCommand(strCmd);
 }
----------------
It should be like:
```
const CMIUtilString strCommand(CMIUtilString::Format("-file-exec-and-symbols \"%s\"", m_strCmdLineArgExecuteableFileNamePath.c_str()));
const bool bOk = CMICmnStreamStdout::TextToStdout(strCommand);
return (bOk && InterpretCommand(strCommand));
```

================
Comment at: tools/lldb-mi/MIDriverMain.cpp:137
@@ -136,3 +137,1 @@
-    // Signal MI to shutdown or halt a running debug session
-    CMICmnStreamStdin::Instance().SetCtrlCHit();
 }
----------------
Should we remove it too?

================
Comment at: tools/lldb-mi/MIDriverMgr.cpp:792
@@ +791,3 @@
+void
+CMIDriverMgr::DeliverSignal(int signal)
+{
----------------
void DeliverSignal(const int vnSignal)

================
Comment at: tools/lldb-mi/MIDriverMgr.h:82
@@ -81,2 +81,3 @@
         virtual const CMIUtilString &GetId(void) const = 0;
+        virtual void DeliverSignal(int) = 0;
 
----------------
Please, specify an argument name and let's use lldb-mi style:
```
virtual void DeliverSignal(const int vnSignal) = 0;
```

================
Comment at: tools/lldb-mi/MIDriverMgr.h:110
@@ -108,2 +109,3 @@
     lldb::SBDebugger *DriverGetTheDebugger(void);
+    void DeliverSignal(int);
 
----------------
void DeliverSignal(const int vnSignal);

http://reviews.llvm.org/D7746

EMAIL PREFERENCES
  http://reviews.llvm.org/settings/panel/emailpreferences/






More information about the lldb-commits mailing list