Resurrection of "Hooks for out-of-process (OOP) debugger" CL.

Resurrection of "Hooks for out-of-process (OOP) debugger" CL.

original CL link:
http://codereview.chromium.org/6995160/
Committed: http://src.chromium.org/viewvc/native_client?view=rev&revision=5666

[garianov1, 2011-06-14 19:55:55]

trybots are green

[Brad Chen, 2011-06-14 19:59:24]

What did you have to change to make things work? It would be helpful
reviewers if you indicated what changed since the last revision.

Brad

On Tue, Jun 14, 2011 at 12:55 PM, <garianov@chromium.org> wrote:

> Reviewers: bsy,
>
> Message:
> trybots are green
>
> Description:
> Resurrection of "Hooks for out-of-process (OOP) debugger" CL.
>
> original CL link:
>
http://codereview.chromium.**org/6995160/<http://codereview.chromium.org/6995...
>
> Please review this at
http://codereview.chromium.**org/7164001/<http://codereview.chromium.org/7164...
>
> SVN Base: svn://svn.chromium.org/native_**client/trunk/src/native_**
> client/ <http://svn.chromium.org/native_client/trunk/src/native_client/>
>
> Affected files:
>  M     src/trusted/service_runtime/**build.scons
>  A     src/trusted/service_runtime/**linux/nacl_oop_debugger_hooks.**c
>  M     src/trusted/service_runtime/**nacl_app_thread.c
>  A     src/trusted/service_runtime/**nacl_oop_debugger_hooks.h
>  A     src/trusted/service_runtime/**osx/nacl_oop_debugger_hooks.c
>  M     src/trusted/service_runtime/**sel_ldr_standard.c
>  M     src/trusted/service_runtime/**service_runtime.gyp
>  A     src/trusted/service_runtime/**win/nacl_oop_debugger_hooks.c
>
>
> --
> You received this message because you are subscribed to the Google Groups
> "Native-Client-Reviews" group.
> To post to this group, send email to native-client-reviews@**
> googlegroups.com <native-client-reviews@googlegroups.com>.
> To unsubscribe from this group, send email to native-client-reviews+**
>
unsubscribe@googlegroups.com<native-client-reviews%2Bunsubscribe@googlegroups...
> .
> For more options, visit this group at http://groups.google.com/**
>
group/native-client-reviews?**hl=en<http://groups.google.com/group/native-client-reviews?hl=en>
> .
>
>

-- 
You received this message because you are subscribed to the Google Groups
"Native-Client-Reviews" group.
To post to this group, send email to native-client-reviews@googlegroups.com.
To unsubscribe from this group, send email to
native-client-reviews+unsubscribe@googlegroups.com.
For more options, visit this group at
http://groups.google.com/group/native-client-reviews?hl=en.

[garianov1, 2011-06-14 20:02:00]

On 2011/06/14 19:59:24, Brad Chen wrote:
> What did you have to change to make things work? It would be helpful
> reviewers if you indicated what changed since the last revision.
> 
> Brad
> 
> On Tue, Jun 14, 2011 at 12:55 PM, <mailto:garianov@chromium.org> wrote:
> 
> > Reviewers: bsy,
> >
> > Message:
> > trybots are green
> >
> > Description:
> > Resurrection of "Hooks for out-of-process (OOP) debugger" CL.
> >
> > original CL link:
> >
>
http://codereview.chromium.**org/6995160/%3Chttp://codereview.chromium.org/69...>
> >
> > Please review this at
>
http://codereview.chromium.**org/7164001/%3Chttp://codereview.chromium.org/71...>
> >
> > SVN Base: svn://svn.chromium.org/native_**client/trunk/src/native_**
> > client/ <http://svn.chromium.org/native_client/trunk/src/native_client/>
> >
> > Affected files:
> >  M     src/trusted/service_runtime/**build.scons
> >  A     src/trusted/service_runtime/**linux/nacl_oop_debugger_hooks.**c
> >  M     src/trusted/service_runtime/**nacl_app_thread.c
> >  A     src/trusted/service_runtime/**nacl_oop_debugger_hooks.h
> >  A     src/trusted/service_runtime/**osx/nacl_oop_debugger_hooks.c
> >  M     src/trusted/service_runtime/**sel_ldr_standard.c
> >  M     src/trusted/service_runtime/**service_runtime.gyp
> >  A     src/trusted/service_runtime/**win/nacl_oop_debugger_hooks.c
> >
> >
> > --
> > You received this message because you are subscribed to the Google Groups
> > "Native-Client-Reviews" group.
> > To post to this group, send email to native-client-reviews@**
> > http://googlegroups.com <mailto:native-client-reviews@googlegroups.com>.
> > To unsubscribe from this group, send email to native-client-reviews+**
> >
>
mailto:unsubscribe@googlegroups.com<native-client-reviews%2Bunsubscribe@googlegroups.com>
> > .
> > For more options, visit this group at http://groups.google.com/**
> >
>
group/native-client-reviews?**hl=en<http://groups.google.com/group/native-client-reviews?hl=en>
> > .
> >
> >
> 
> -- 
> You received this message because you are subscribed to the Google Groups
> "Native-Client-Reviews" group.
> To post to this group, send email to
mailto:native-client-reviews@googlegroups.com.
> To unsubscribe from this group, send email to
> mailto:native-client-reviews+unsubscribe@googlegroups.com.
> For more options, visit this group at
> http://groups.google.com/group/native-client-reviews?hl=en.
> 

Changes are in linux & mac os stubs (nacl_oop_debugger_hook.c),
they needed 
#include "native_client/src/include/nacl_platform.h"
to get definition of UNREFERENCED_PARAMETER macro.

[bsy, 2011-06-15 00:28:04]

same nit as last time.

http://codereview.chromium.org/7164001/diff/5001/src/trusted/service_runtime/...
File src/trusted/service_runtime/win/nacl_oop_debugger_hooks.c (right):

http://codereview.chromium.org/7164001/diff/5001/src/trusted/service_runtime/...
src/trusted/service_runtime/win/nacl_oop_debugger_hooks.c:89: }
== -1 case when fmt is NULL?

[garianov1, 2011-06-15 19:17:49]

http://codereview.chromium.org/7164001/diff/5001/src/trusted/service_runtime/...
File src/trusted/service_runtime/win/nacl_oop_debugger_hooks.c (right):

http://codereview.chromium.org/7164001/diff/5001/src/trusted/service_runtime/...
src/trusted/service_runtime/win/nacl_oop_debugger_hooks.c:89: }
On 2011/06/15 00:28:04, bsy wrote:
> == -1 case when fmt is NULL?

I have a check for NULL in the beginning of the function.
Adding LOG_FATAL here too...

[bsy, 2011-06-15 19:23:12]

lgtm.  sorry didn't see the fmt NULL check.

[garianov1, 2011-06-15 21:01:18]

On 2011/06/15 19:23:12, bsy wrote:
> lgtm.  sorry didn't see the fmt NULL check.

committed as rev 5666

[garianov1, 2011-06-16 16:56:31]

On 2011/06/15 21:01:18, garianov1 wrote:
> On 2011/06/15 19:23:12, bsy wrote:
> > lgtm.  sorry didn't see the fmt NULL check.
> 
> committed as rev 5666

Bennet, thanks for the prompt review!