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

[Jorge Gorbe Moya via Phabricator via lldb-commits]

jgorbe added inline comments.


================
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()};
----------------
rupprecht wrote:
> Only pass this if != 0
> 
> (or get fancy and use a std::optional or whatever)
Done. Given that I've changed the `debugger_pid` argument to an `lldb::pid_t` I'm using `LLDB_INVALID_PROCESS_ID` as a sentinel value.


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


================
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') {
----------------
rupprecht wrote:
> `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() << ...
>       }
>     }
> ```
Thanks! Once this patch lands we should consider changing how the `port` flag is parsed (just a few lines after this block) to match this style.


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

https://reviews.llvm.org/D147805