<div dir="ltr">Makes sense. I think the handle makes more sense in the ProcessInstanceInfo than the ProcessInfo, since by definition a HANDLE implies that you're referring to an instance of the process that is currently running, which is what ProcessInstanceInfo is for. <div>
<br></div><div>Still though, I wonder if it might be better to have LaunchProcess take a ProcessInstanceInfo* argument that could be filled out by the function. I agree that just putting the lldb::host::process_t directly into the ProcessLaunchInfo alongside the pid would be easier, but I don't mind doing the extra work for a more robust solution, if you agree that there's value in doing it this way.</div>
</div><div class="gmail_extra"><br><br><div class="gmail_quote">On Tue, Aug 12, 2014 at 4:09 PM, Greg Clayton <span dir="ltr"><<a href="mailto:gclayton@apple.com" target="_blank">gclayton@apple.com</a>></span> wrote:<br>
<blockquote class="gmail_quote" style="margin:0 0 0 .8ex;border-left:1px #ccc solid;padding-left:1ex"><div class=""><br>
> On Aug 12, 2014, at 3:58 PM, Zachary Turner <<a href="mailto:zturner@google.com">zturner@google.com</a>> wrote:<br>
><br>
> Thanks, that makes sense. The problem is that Host::LaunchProcess doesn't really return anything useful. When I create a process on Windows, I get a HANDLE back that I can use to later manipulate the process. Note that this isn't the same as a Pid. I guess Host::LaunchProcess will return the pid in the launch_info structure,<br>
<br>
</div>Indeed it does.<br>
<div class=""><br>
> which I could then use to get another handle, but that would involve closing the first handle before I get the second handle, which I'm a little wary of, and I would feel better just using the original handle. But there's no obvious way to pass it back.<br>
<br>
</div>We can have the LaunchInfo contain the new "host" version of the process handle which would need to be added to the lldb-types.h. We had spoke about this before about possibly adding a:<br>
<br>
namespace lldb {<br>
namespace host {<br>
typedef pid_t process_t;<br>
}<br>
}<br>
<br>
On windows you could make this a process handle and add lldb_private::ProcessInfo could have a new ivar added:<br>
<br>
namespace lldb_private<br>
{<br>
class ProcessInfo<br>
{<br>
<br>
FileSpec m_executable;<br>
std::string m_arg0; // argv[0] if supported. If empty, then use m_executable.<br>
// Not all process plug-ins support specifying an argv[0]<br>
// that differs from the resolved platform executable<br>
// (which is in m_executable)<br>
Args m_arguments; // All program arguments except argv[0]<br>
Args m_environment;<br>
uint32_t m_uid;<br>
uint32_t m_gid;<br>
ArchSpec m_arch;<br>
lldb::pid_t m_pid;<br>
lldb::host::process_t m_process_handle; // <<<<< new ivar<br>
<div class=""> };<br>
<br>
><br>
> I'm not sure what the right abstraction is here that would make sense on all platforms, but I feel like what is needed is some kind of NativeProcess class, and LaunchProcess's signature could change to this:<br>
<br>
</div>I believe the lldb::host::process_t should be enough. It can be a class if needed for your system, or it can just be a quick typedef like above.<br>
<div class=""><br>
><br>
> static Error<br>
> Host::LaunchProcess (ProcessLaunchInfo &launch_info, NativeProcess **process);<br>
><br>
> If the user passes in NULL, then we just clean up the process handle (or do nothing, depending on platform), and if it's not NULL, then Host allocates the appropriate type of Platform specific Process object. For example, it allocates a NativeProcessLinux, or a NativeProcessMacOSX, or a NativeProcessWindows, etc.<br>
><br>
> Do you think something like this could work? Or maybe have other ideas?<br>
<br>
</div>I would update the ProcessInfo (ProcessInstanceInfo inherits from it) since this also ties into the platform functions that find and get info for processes:<br>
<br>
<br>
virtual uint32_t<br>
Platform::FindProcesses (const ProcessInstanceInfoMatch &match_info,<br>
ProcessInstanceInfoList &proc_infos);<br>
<br>
virtual bool<br>
Platform::GetProcessInfo (lldb::pid_t pid, ProcessInstanceInfo &proc_info);<br>
<br>
And it would be nice to get these native process handles there as well.<br>
<span class="HOEnZb"><font color="#888888"><br>
Greg<br>
</font></span></blockquote></div><br></div>