clayborg requested changes to this revision.
clayborg added a comment.
This revision now requires changes to proceed.

Very close, just need to check for the "host" platform and possibly add windows 
support, or return an error on windows, if LLVM doesn't have a OS abstracted 
wrapper that allows access to the current environment.



================
Comment at: 
lldb/test/API/tools/lldb-vscode/environmentVariables/TestVSCode_environmentVariables.py:2
+"""
+Test lldb-vscode completions request
+"""
----------------
This comment seems to need fixing, probably due to copying from a previous test 
case file.


================
Comment at: lldb/tools/lldb-vscode/lldb-vscode.cpp:52
 
+extern char **environ;
+
----------------
Will this work on windows or non unix systems? Are there any llvm functions 
that get the environment that we might be able to use?


================
Comment at: lldb/tools/lldb-vscode/lldb-vscode.cpp:1376
   auto envs = GetStrings(arguments, "env");
+  if (launchWithDebuggerEnvironment) {
+    char** env_var_pointer = environ;
----------------
I think we need to grab the platform form the target and see if the platform is 
the "host" platform here and error out if we catch someone trying to forward an 
environment to another remote platform. Maybe something like:
```
if (launchWithDebuggerEnvironment) {
  const char *platform_name = g_vsc.target.GetPlatform().GetName();
  if (platform_name && strcmp(platform_name, "host") != 0) {
    response["success"] = llvm::json::Value(false);
    EmplaceSafeString(response, "message", "can't use inheritEnvironment on a 
remote process");
    g_vsc.SendJSON(llvm::json::Value(std::move(response)));
    return;
  }
}
```
    


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D74636



_______________________________________________
lldb-commits mailing list
lldb-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/lldb-commits

Reply via email to