[Lldb-commits] [PATCH] D41070: llgs: Propagate the environment when launching the inferior from command line

[Greg Clayton via Phabricator via lldb-commits]

clayborg accepted this revision.
clayborg added a comment.
This revision is now accepted and ready to land.

Anything that launches a process should use the ProcessLaunchInfo. Nice patch.



================
Comment at: source/Plugins/Process/gdb-remote/GDBRemoteCommunicationServerLLGS.h:46
 
-  //------------------------------------------------------------------
-  /// Specify the program to launch and its arguments.
-  ///
-  /// @param[in] args
-  ///     The command line to launch.
-  ///
-  /// @param[in] argc
-  ///     The number of elements in the args array of cstring pointers.
-  ///
-  /// @return
-  ///     An Status object indicating the success or failure of making
-  ///     the setting.
-  //------------------------------------------------------------------
-  Status SetLaunchArguments(const char *const args[], int argc);
-
-  //------------------------------------------------------------------
-  /// Specify the launch flags for the process.
-  ///
-  /// @param[in] launch_flags
-  ///     The launch flags to use when launching this process.
-  ///
-  /// @return
-  ///     An Status object indicating the success or failure of making
-  ///     the setting.
-  //------------------------------------------------------------------
-  Status SetLaunchFlags(unsigned int launch_flags);
+  void SetLaunchInfo(const ProcessLaunchInfo &info);
 
----------------
We might want a "ProcessLaunchInfo &" accessor so we can modify the flags after they have initially been set?

```
ProcessLaunchInfo &GetLaunchInfo() { return m_process_launch_info; }
```

If we don't need it, we shouldn't add it yet, just thinking out loud here.


================
Comment at: tools/lldb-server/lldb-gdbserver.cpp:181-182
+  ProcessLaunchInfo info;
+  info.GetFlags().Set(eLaunchFlagStopAtEntry | eLaunchFlagDebug |
+                      eLaunchFlagDisableASLR);
+  info.SetArguments(const_cast<const char **>(argv), true);
----------------
How many places do we set these flags like this? Wondering if we should add a method like:

```
info.SetFlagsForDebugging();
```

That way if we add new flags that must be set later, it will be easier than updating all sites that modify these flags for debugging a process.


https://reviews.llvm.org/D41070