[Lldb-commits] [PATCH] D147805: [lldb-vscode] Fix two issues with runInTerminal test.

[Jordan Rupprecht via Phabricator via lldb-commits]

rupprecht added a reviewer: rupprecht.
rupprecht added a comment.

The `pid` plumbing looks fine for the happy case, but I think we could be more lenient if (for whatever reason) the pid flag isn't being set on non-Linux systems that won't actually be using it. Even on a Linux system, this is only needed if ptrace_scope != 0, so if the `pid` isn't being provided there, we could just skip the `prctl` call and hope the user's system has a value of 0 for that.

(Also, 0 is a weird value, we should probably just reject that)



================
Comment at: lldb/tools/lldb-vscode/JSONUtils.cpp:1120
       debug_adaptor_path.str(), "--comm-file", comm_file.str(),
+      "--debugger-pid", std::to_string(debugger_pid),
       "--launch-target", GetString(launch_request_arguments, "program").str()};
----------------
Only pass this if != 0

(or get fancy and use a std::optional or whatever)


================
Comment at: lldb/tools/lldb-vscode/lldb-vscode.cpp:1569
 
+  unsigned long debugger_pid = 0;
+#if !defined(_WIN32)
----------------
unsigned long -> lldb::pid_t


================
Comment at: lldb/tools/lldb-vscode/lldb-vscode.cpp:3163
+#if defined(__linux__)
+  (void)prctl(PR_SET_PTRACER, debugger_pid, 0, 0, 0);
+#endif
----------------
Only invoke this if debugger_pid != 0


================
Comment at: lldb/tools/lldb-vscode/lldb-vscode.cpp:3261
+      char *remainder;
+      unsigned long pid = strtoul(debugger_pid->getValue(), &remainder, 0);
+      if (remainder == debugger_pid->getValue() || *remainder != '\0') {
----------------
`StringRef` is usually used for parsing strings as numbers, something like:

```
    unsigned long pid = 0;
      llvm::StringRef debugger_pid_value = debugger_pid->getValue())
      if (debugger_pid_value.getAsInteger(10, pid)) {
        llvm::errs() << ...
      }
    }
```


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

https://reviews.llvm.org/D147805