[lldb-dev] Host::LaunchProcess vs. ProcessPlugin::DoLaunch

[Zachary Turner]

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.

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.


On Tue, Aug 12, 2014 at 4:09 PM, Greg Clayton <gclayton at apple.com> wrote:

>
> > On Aug 12, 2014, at 3:58 PM, Zachary Turner <zturner at google.com> wrote:
> >
> > 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,
>
> Indeed it does.
>
> > 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.
>
> 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:
>
> namespace lldb {
>   namespace host {
>     typedef pid_t process_t;
>   }
> }
>
> On windows you could make this a process handle and add
> lldb_private::ProcessInfo could have a new ivar added:
>
> namespace lldb_private
> {
>     class ProcessInfo
>     {
>
>         FileSpec m_executable;
>         std::string m_arg0; // argv[0] if supported. If empty, then use
> m_executable.
>         // Not all process plug-ins support specifying an argv[0]
>         // that differs from the resolved platform executable
>         // (which is in m_executable)
>         Args m_arguments;   // All program arguments except argv[0]
>         Args m_environment;
>         uint32_t m_uid;
>         uint32_t m_gid;
>         ArchSpec m_arch;
>         lldb::pid_t m_pid;
>         lldb::host::process_t m_process_handle; // <<<<< new ivar
>     };
>
> >
> > 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:
>
> 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.
>
> >
> > static Error
> > Host::LaunchProcess (ProcessLaunchInfo &launch_info, NativeProcess
> **process);
> >
> > 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.
> >
> > Do you think something like this could work?  Or maybe have other ideas?
>
> I would update the ProcessInfo (ProcessInstanceInfo inherits from it)
> since this also ties into the platform functions that find and get info for
> processes:
>
>
>         virtual uint32_t
>         Platform::FindProcesses (const ProcessInstanceInfoMatch
> &match_info,
>                                  ProcessInstanceInfoList &proc_infos);
>
>         virtual bool
>         Platform::GetProcessInfo (lldb::pid_t pid, ProcessInstanceInfo
> &proc_info);
>
> And it would be nice to get these native process handles there as well.
>
> Greg
>
-------------- next part --------------
An HTML attachment was scrubbed...
URL: <http://lists.llvm.org/pipermail/lldb-dev/attachments/20140812/41e1aafe/attachment.html>