[Lldb-commits] [PATCH] Introduce a ProcessStatus monitor abstraction.

[Zachary Turner]

On Thu Oct 30 2014 at 1:21:24 PM Greg Clayton <clayborg at gmail.com> wrote:

> In include/lldb/Host/MonitoringProcessLauncher.h: please include
> lldb-forward.h and don't forward declare anything manually
>
> In explicit MonitoringProcessLauncher(lldb::ProcessStatusMonitorSP
> monitor, lldb::ProcessLauncherUP delegate_launcher); I would change each
> arg to be a const reference to avoid copies and reference counts being
> bumped.
>
The first argument, the monitor, can't be passed by reference because null
is a valid value.  This is how the fallback to current behavior works for
non-Windows platforms.  If you give it a non-null monitor it will use
that.  If you give it null, it will call
Host::StartMonitoringChildProcess.  Ultimately you could create a
PosixSpawnMonitor or something and copy the code from
Host::StartMonitoringChildProcess into that class, but I didn't want to
turn this into a mega-refactor.  But that would be the ultimate goal, and
it would also make the monitoring algorithm more re-usable so that, for
example, llgs could use it.




>
> Change ProcessStatusMonitorCallbackEntry to not use std::pair and declare
> a small structure instead. Seeing "a.first" and "a.second" really doesn't
> help the readability of the code, so please make a struct with good member
> names ("callback" and "baton").
>
> In lldb-forward.h the spacing of the new stuff you added seems to be using
> tabs as it isn't indented the same as the others. Please make sure to use
> spaces instead of tabs.
>
> http://reviews.llvm.org/D6036
>
>
>
-------------- next part --------------
An HTML attachment was scrubbed...
URL: <http://lists.llvm.org/pipermail/lldb-commits/attachments/20141030/47987c61/attachment.html>