<div class="gmail_quote">On Thu Oct 30 2014 at 1:21:24 PM Greg Clayton <<a href="mailto:clayborg@gmail.com">clayborg@gmail.com</a>> wrote:<br><blockquote class="gmail_quote" style="margin:0 0 0 .8ex;border-left:1px #ccc solid;padding-left:1ex">In include/lldb/Host/<u></u>MonitoringProcessLauncher.h: please include lldb-forward.h and don't forward declare anything manually<br>
<br>
In explicit MonitoringProcessLauncher(<u></u>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.<br></blockquote><div><span style="font-size:13.1999998092651px">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.</span><br></div><div><span style="font-size:13.1999998092651px"><br></span></div><div><span style="font-size:13.1999998092651px"><br></span></div><div> </div><blockquote class="gmail_quote" style="margin:0 0 0 .8ex;border-left:1px #ccc solid;padding-left:1ex">
<br>
Change ProcessStatusMonitorCallbackEn<u></u>try 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").<br>
<br>
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.<br>
<br>
<a href="http://reviews.llvm.org/D6036" target="_blank">http://reviews.llvm.org/D6036</a><br>
<br>
<br>
</blockquote></div>