[Lldb-commits] [PATCH] Fix TestProcessIO.py when run against a remote target

Greg Clayton clayborg at gmail.com
Wed Feb 11 13:37:37 PST 2015


See my inlined code comments.


================
Comment at: source/Plugins/Process/gdb-remote/GDBRemoteCommunicationServer.cpp:503
@@ +502,3 @@
+{
+    if (InferiorStdoutOrStderrOverGdbRemote(launch_info))
+    {
----------------
Should this be:


```
if (launch_info.GetFileActionForFD (STDOUT_FILENO) == nullptr &&
    launch_info.GetFileActionForFD (STDERR_FILENO) == nullptr)
```

================
Comment at: source/Plugins/Process/gdb-remote/GDBRemoteCommunicationServer.cpp:525-543
@@ -502,19 +524,21 @@
+
+bool
+GDBRemoteCommunicationServer::InferiorStdoutOrStderrOverGdbRemote (const lldb_private::ProcessLaunchInfo &launch_info) const
 {
     // Retrieve the file actions specified for stdout and stderr.
     auto stdout_file_action = launch_info.GetFileActionForFD (STDOUT_FILENO);
     auto stderr_file_action = launch_info.GetFileActionForFD (STDERR_FILENO);
 
     // If neither stdout and stderr file actions are specified, we're not doing anything special, so
     // assume we want to redirect stdout/stderr over gdb-remote $O messages.
-    if ((stdout_file_action == nullptr) && (stderr_file_action == nullptr))
+    if (stdout_file_action == nullptr || stderr_file_action == nullptr)
     {
         // Send stdout/stderr over the gdb-remote protocol.
         return true;
     }
 
-    // Any other setting for either stdout or stderr implies we are either suppressing
-    // it (with /dev/null) or we've got it set to a PTY.  Either way, we don't want the
-    // output over gdb-remote.
+    // no stdout or stderr over gdb remote
+    // it's either disabled, or redirected to a file
     return false;
 }
 
----------------
Remove this in favor of just inlining the following code? 

```
if (launch_info.GetFileActionForFD (STDOUT_FILENO) == nullptr &&
    launch_info.GetFileActionForFD (STDERR_FILENO) == nullptr)
```

thoughts? You could add new functions to ProcessLaunch if you want to make it:


```
if (!launch_info.HasFileActionFor(STDOUT_FILENO) && !launch_info.HasFileAction(STDERR_FILENO))
```

Or if you added a var arg version:


```
if (!launch_info.HasFileActionForAnyOf(STDOUT_FILENO, STDERR_FILENO))
```

================
Comment at: source/Plugins/Process/gdb-remote/GDBRemoteCommunicationServer.cpp:572
@@ -547,3 +571,3 @@
     // stdout/stderr over the gdb-remote protocol.
-    if (ShouldRedirectInferiorOutputOverGdbRemote (m_process_launch_info))
+    if (InferiorStdioOverGdbRemote (m_process_launch_info))
     {
----------------
Inline accessors to m_process_launch_info as state above?

================
Comment at: source/Plugins/Process/gdb-remote/GDBRemoteCommunicationServer.cpp:1897
@@ -1869,2 +1896,3 @@
 }
+
 bool
----------------
Get rid of this empty line diff?

http://reviews.llvm.org/D7547

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






More information about the lldb-commits mailing list