Non-SFI mode: Implement nacl_irt_random only for nacl_helper_nonsfi.

Non-SFI mode: Implement nacl_irt_random only for nacl_helper_nonsfi.

In order to share some startup functions with SFI mode, especially
around IRT implementations, this CL implements nacl_irt_random.
The original implementation is components/nacl/loader/nonsfi/irt_random.cc.

BUG=http://code.google.com/p/chromium/issues/detail?id=358465
TEST=Ran trybot. Implement nacl_helper_nonsfi on top of this CL and
made sure its browser_tests pass.

Committed: http://src.chromium.org/viewvc/native_client?view=rev&revision=14056

[hidehiko, 2014-10-28 20:17:32]

hidehiko@chromium.org changed reviewers:
+ hamaji@chromium.org, mazda@chromium.org, mseaborn@chromium.org,
uekawa@chromium.org

[hidehiko, 2014-10-28 20:17:33]

This should help sharing chrome_irt_query function in irt_ppapi.cc between SFI
and Non-SFI. (I.e., with this CL, we will be able to use chrome_irt_query from
nonsfi_main.cc directly, too.).

The concept of urandom fd initialization and IRT adding condition strategy is
similar to the irt_exception_handling.h.

Thank you for your review in advance,
- hidehiko

[hidehiko, 2014-10-29 16:57:54]

On 2014/10/28 20:17:33, hidehiko wrote:
> This should help sharing chrome_irt_query function in irt_ppapi.cc between SFI
> and Non-SFI. (I.e., with this CL, we will be able to use chrome_irt_query from
> nonsfi_main.cc directly, too.).
> 
> The concept of urandom fd initialization and IRT adding condition strategy is
> similar to the irt_exception_handling.h.
> 
> Thank you for your review in advance,
> - hidehiko

Friendly ping?

[Junichi Uekawa, 2014-10-30 03:46:33]

On 2014/10/29 16:57:54, hidehiko wrote:
> On 2014/10/28 20:17:33, hidehiko wrote:
> > This should help sharing chrome_irt_query function in irt_ppapi.cc between
SFI
> > and Non-SFI. (I.e., with this CL, we will be able to use chrome_irt_query
from
> > nonsfi_main.cc directly, too.).
> > 
> > The concept of urandom fd initialization and IRT adding condition strategy
is
> > similar to the irt_exception_handling.h.
> > 
> > Thank you for your review in advance,
> > - hidehiko
> 
> Friendly ping?

There is going to be a matching change in
components/nacl/loader/nonsfi/nonsfi_listener.cc ?

[Junichi Uekawa, 2014-10-30 03:46:35]

On 2014/10/29 16:57:54, hidehiko wrote:
> On 2014/10/28 20:17:33, hidehiko wrote:
> > This should help sharing chrome_irt_query function in irt_ppapi.cc between
SFI
> > and Non-SFI. (I.e., with this CL, we will be able to use chrome_irt_query
from
> > nonsfi_main.cc directly, too.).
> > 
> > The concept of urandom fd initialization and IRT adding condition strategy
is
> > similar to the irt_exception_handling.h.
> > 
> > Thank you for your review in advance,
> > - hidehiko
> 
> Friendly ping?

There is going to be a matching change in
components/nacl/loader/nonsfi/nonsfi_listener.cc ?

[hidehiko, 2014-10-30 04:20:05]

On 2014/10/30 03:46:35, Junichi Uekawa wrote:
> On 2014/10/29 16:57:54, hidehiko wrote:
> > On 2014/10/28 20:17:33, hidehiko wrote:
> > > This should help sharing chrome_irt_query function in irt_ppapi.cc between
> SFI
> > > and Non-SFI. (I.e., with this CL, we will be able to use chrome_irt_query
> from
> > > nonsfi_main.cc directly, too.).
> > > 
> > > The concept of urandom fd initialization and IRT adding condition strategy
> is
> > > similar to the irt_exception_handling.h.
> > > 
> > > Thank you for your review in advance,
> > > - hidehiko
> > 
> > Friendly ping?
> 
> There is going to be a matching change in
> components/nacl/loader/nonsfi/nonsfi_listener.cc ?

Yes, I'll make some change in nonsfi_listener.cc, like

#if defined(OS_NACL_NONSFI)
  nonsfi_set_urandom_fd(...);
#else
  nacl::nonsfi::SetUrandomFd(...);
#endif

It'll be done a part of the CL for nacl_helper_nonsfi implementation.

[Junichi Uekawa, 2014-10-30 11:04:50]

lgtm

[Mark Seaborn, 2014-10-30 18:58:31]

https://codereview.chromium.org/686723003/diff/1/src/nonsfi/irt/irt_interfaces.c
File src/nonsfi/irt/irt_interfaces.c (right):

https://codereview.chromium.org/686723003/diff/1/src/nonsfi/irt/irt_interface...
src/nonsfi/irt/irt_interfaces.c:439: static int urandom_fd = -1;
Style nit: Add "g_" prefix to the name

https://codereview.chromium.org/686723003/diff/1/src/nonsfi/irt/irt_interface...
src/nonsfi/irt/irt_interfaces.c:445: static int irt_get_random_bytes(void *buf,
size_t count, size_t *nread) {
It's rather unfortunate that this isn't tested on the NaCl side.

One of the goals in the newlib switch was to improve testing of IRT interface
implementations, because moving IRT interface implementations to the NaCl side
allows us to run NaCl-side tests against those implementations.

So, could you add a small amount of plumbing so that we can enable
run_random_test_irt to cover this code?  i.e. If urandom_fd == -1, try to open
/dev/urandom.

(An alternative would be to implement irt_random on the Chromium side.  That
would be reasonable, since irt_random is very simple to implement.  However,
implementing irt_random on the NaCl side is more usable since the code can be
reused for the unsandboxed-PNaCl use case.)

[hidehiko, 2014-10-31 14:03:32]

Patchset #2 (id:20001) has been deleted

[hidehiko, 2014-10-31 14:03:37]

Patchset #2 (id:40001) has been deleted

[hidehiko, 2014-10-31 14:03:52]

Thank you for review. PTAL.

https://codereview.chromium.org/686723003/diff/1/src/nonsfi/irt/irt_interfaces.c
File src/nonsfi/irt/irt_interfaces.c (right):

https://codereview.chromium.org/686723003/diff/1/src/nonsfi/irt/irt_interface...
src/nonsfi/irt/irt_interfaces.c:439: static int urandom_fd = -1;
On 2014/10/30 18:58:30, Mark Seaborn wrote:
> Style nit: Add "g_" prefix to the name

Done (in irt_random.c).

https://codereview.chromium.org/686723003/diff/1/src/nonsfi/irt/irt_interface...
src/nonsfi/irt/irt_interfaces.c:445: static int irt_get_random_bytes(void *buf,
size_t count, size_t *nread) {
On 2014/10/30 18:58:30, Mark Seaborn wrote:
> It's rather unfortunate that this isn't tested on the NaCl side.
> 
> One of the goals in the newlib switch was to improve testing of IRT interface
> implementations, because moving IRT interface implementations to the NaCl side
> allows us to run NaCl-side tests against those implementations.
> 
> So, could you add a small amount of plumbing so that we can enable
> run_random_test_irt to cover this code?  i.e. If urandom_fd == -1, try to open
> /dev/urandom.
> 
> (An alternative would be to implement irt_random on the Chromium side.  That
> would be reasonable, since irt_random is very simple to implement.  However,
> implementing irt_random on the NaCl side is more usable since the code can be
> reused for the unsandboxed-PNaCl use case.)

Done.

[hidehiko, 2014-11-03 13:32:06]

Friendly ping?
- hidehiko

On 2014/10/31 14:03:52, hidehiko wrote:
> Thank you for review. PTAL.
> 
>
https://codereview.chromium.org/686723003/diff/1/src/nonsfi/irt/irt_interfaces.c
> File src/nonsfi/irt/irt_interfaces.c (right):
> 
>
https://codereview.chromium.org/686723003/diff/1/src/nonsfi/irt/irt_interface...
> src/nonsfi/irt/irt_interfaces.c:439: static int urandom_fd = -1;
> On 2014/10/30 18:58:30, Mark Seaborn wrote:
> > Style nit: Add "g_" prefix to the name
> 
> Done (in irt_random.c).
> 
>
https://codereview.chromium.org/686723003/diff/1/src/nonsfi/irt/irt_interface...
> src/nonsfi/irt/irt_interfaces.c:445: static int irt_get_random_bytes(void
*buf,
> size_t count, size_t *nread) {
> On 2014/10/30 18:58:30, Mark Seaborn wrote:
> > It's rather unfortunate that this isn't tested on the NaCl side.
> > 
> > One of the goals in the newlib switch was to improve testing of IRT
interface
> > implementations, because moving IRT interface implementations to the NaCl
side
> > allows us to run NaCl-side tests against those implementations.
> > 
> > So, could you add a small amount of plumbing so that we can enable
> > run_random_test_irt to cover this code?  i.e. If urandom_fd == -1, try to
open
> > /dev/urandom.
> > 
> > (An alternative would be to implement irt_random on the Chromium side.  That
> > would be reasonable, since irt_random is very simple to implement.  However,
> > implementing irt_random on the NaCl side is more usable since the code can
be
> > reused for the unsandboxed-PNaCl use case.)
> 
> Done.

[Mark Seaborn, 2014-11-04 01:19:53]

LGTM

https://codereview.chromium.org/686723003/diff/60001/src/nonsfi/irt/build.scons
File src/nonsfi/irt/build.scons (right):

https://codereview.chromium.org/686723003/diff/60001/src/nonsfi/irt/build.sco...
src/nonsfi/irt/build.scons:25: # For testing purpose, we split irt_random from
libirt_nonsfi.a,
Nit: "purpose" -> "purposes"

https://codereview.chromium.org/686723003/diff/60001/src/nonsfi/irt/build.sco...
src/nonsfi/irt/build.scons:26: # which will be linked with the random_test.
Nit: "linked into random_test"?

https://codereview.chromium.org/686723003/diff/60001/src/nonsfi/irt/irt.gyp
File src/nonsfi/irt/irt.gyp (right):

https://codereview.chromium.org/686723003/diff/60001/src/nonsfi/irt/irt.gyp#n...
src/nonsfi/irt/irt.gyp:23: '../../untrusted/nacl/nacl_random.h',
You don't need to list headers in Gyp (especially cross-directory ones). 
Listing headers only affects what appears in an IDE, which is a deprecated use
case for Gyp (since everyone is switching to Ninja), and we've never cared about
this use case in the NaCl repo.

https://codereview.chromium.org/686723003/diff/60001/src/nonsfi/irt/irt_inter...
File src/nonsfi/irt/irt_interfaces.c (right):

https://codereview.chromium.org/686723003/diff/60001/src/nonsfi/irt/irt_inter...
src/nonsfi/irt/irt_interfaces.c:435: 
Nit: don't add empty line here

https://codereview.chromium.org/686723003/diff/60001/src/nonsfi/irt/irt_inter...
src/nonsfi/irt/irt_interfaces.c:604: const struct nacl_irt_random
nacl_irt_random = {
Please run the PNaCl toolchain trybots to check that this works there.

I suspect you need to update toolchain_build/pnacl_targetlibs.py to avoid
breaking the test for unsandboxed-PNaCl mode.

https://codereview.chromium.org/686723003/diff/60001/src/nonsfi/irt/irt_random.c
File src/nonsfi/irt/irt_random.c (right):

https://codereview.chromium.org/686723003/diff/60001/src/nonsfi/irt/irt_rando...
src/nonsfi/irt/irt_random.c:11: #include <errno.h>
Nit: sort #includes

https://codereview.chromium.org/686723003/diff/60001/src/nonsfi/irt/irt_rando...
src/nonsfi/irt/irt_random.c:17: EXTERN_C_BEGIN
Not needed in a .c file.

https://codereview.chromium.org/686723003/diff/60001/src/nonsfi/irt/irt_rando...
src/nonsfi/irt/irt_random.c:26: * Note: This library does not provide
nacl_secure_random_init(), as it is
Er, this library does provide nacl_secure_random_init().  Do you mean it doesn't
need to provide it?

https://codereview.chromium.org/686723003/diff/60001/src/nonsfi/irt/irt_rando...
src/nonsfi/irt/irt_random.c:52: EXTERN_C_END
Ditto

https://codereview.chromium.org/686723003/diff/60001/src/nonsfi/irt/irt_random.h
File src/nonsfi/irt/irt_random.h (right):

https://codereview.chromium.org/686723003/diff/60001/src/nonsfi/irt/irt_rando...
src/nonsfi/irt/irt_random.h:14: /* Initializes the urandom fd by the given |fd|.
*/
"by" -> "with".  Please comment that this takes ownership of the given FD.

https://codereview.chromium.org/686723003/diff/60001/src/untrusted/nacl/nacl....
File src/untrusted/nacl/nacl.scons (right):

https://codereview.chromium.org/686723003/diff/60001/src/untrusted/nacl/nacl....
src/untrusted/nacl/nacl.scons:191: if not env.Bit('posix') and not
env.Bit('nonsfi_nacl'):
Should just be "if not env.Bit('nonsfi_nacl')" to match libnacl_sys_private
below.

[hidehiko, 2014-11-04 07:42:58]

Patchset #4 (id:100001) has been deleted

[hidehiko, 2014-11-04 07:43:04]

Patchset #4 (id:120001) has been deleted

[hidehiko, 2014-11-04 11:16:59]

Thank you for review!

I'll submit once trybots are recovered.

https://codereview.chromium.org/686723003/diff/60001/src/nonsfi/irt/build.scons
File src/nonsfi/irt/build.scons (right):

https://codereview.chromium.org/686723003/diff/60001/src/nonsfi/irt/build.sco...
src/nonsfi/irt/build.scons:25: # For testing purpose, we split irt_random from
libirt_nonsfi.a,
On 2014/11/04 01:19:52, Mark Seaborn wrote:
> Nit: "purpose" -> "purposes"

Done.

https://codereview.chromium.org/686723003/diff/60001/src/nonsfi/irt/build.sco...
src/nonsfi/irt/build.scons:26: # which will be linked with the random_test.
On 2014/11/04 01:19:52, Mark Seaborn wrote:
> Nit: "linked into random_test"?

Done.

https://codereview.chromium.org/686723003/diff/60001/src/nonsfi/irt/irt.gyp
File src/nonsfi/irt/irt.gyp (right):

https://codereview.chromium.org/686723003/diff/60001/src/nonsfi/irt/irt.gyp#n...
src/nonsfi/irt/irt.gyp:23: '../../untrusted/nacl/nacl_random.h',
On 2014/11/04 01:19:52, Mark Seaborn wrote:
> You don't need to list headers in Gyp (especially cross-directory ones). 
> Listing headers only affects what appears in an IDE, which is a deprecated use
> case for Gyp (since everyone is switching to Ninja), and we've never cared
about
> this use case in the NaCl repo.

Removed .h files from this list.

https://codereview.chromium.org/686723003/diff/60001/src/nonsfi/irt/irt_inter...
File src/nonsfi/irt/irt_interfaces.c (right):

https://codereview.chromium.org/686723003/diff/60001/src/nonsfi/irt/irt_inter...
src/nonsfi/irt/irt_interfaces.c:435: 
On 2014/11/04 01:19:52, Mark Seaborn wrote:
> Nit: don't add empty line here

Oops. Removed.

https://codereview.chromium.org/686723003/diff/60001/src/nonsfi/irt/irt_inter...
src/nonsfi/irt/irt_interfaces.c:604: const struct nacl_irt_random
nacl_irt_random = {
On 2014/11/04 01:19:52, Mark Seaborn wrote:
> Please run the PNaCl toolchain trybots to check that this works there.
> 
> I suspect you need to update toolchain_build/pnacl_targetlibs.py to avoid
> breaking the test for unsandboxed-PNaCl mode.

Good catch! Fixed.

https://codereview.chromium.org/686723003/diff/60001/src/nonsfi/irt/irt_random.c
File src/nonsfi/irt/irt_random.c (right):

https://codereview.chromium.org/686723003/diff/60001/src/nonsfi/irt/irt_rando...
src/nonsfi/irt/irt_random.c:11: #include <errno.h>
On 2014/11/04 01:19:52, Mark Seaborn wrote:
> Nit: sort #includes

Done.

https://codereview.chromium.org/686723003/diff/60001/src/nonsfi/irt/irt_rando...
src/nonsfi/irt/irt_random.c:17: EXTERN_C_BEGIN
On 2014/11/04 01:19:52, Mark Seaborn wrote:
> Not needed in a .c file.

Done.

https://codereview.chromium.org/686723003/diff/60001/src/nonsfi/irt/irt_rando...
src/nonsfi/irt/irt_random.c:26: * Note: This library does not provide
nacl_secure_random_init(), as it is
On 2014/11/04 01:19:52, Mark Seaborn wrote:
> Er, this library does provide nacl_secure_random_init().  Do you mean it
doesn't
> need to provide it?

Oops, sorry. This was wrong comment. Initially I thought it was not needed, but
it turned out we actually need it for testing purpose. Updated the comment.

https://codereview.chromium.org/686723003/diff/60001/src/nonsfi/irt/irt_rando...
src/nonsfi/irt/irt_random.c:52: EXTERN_C_END
On 2014/11/04 01:19:52, Mark Seaborn wrote:
> Ditto

Done.

https://codereview.chromium.org/686723003/diff/60001/src/nonsfi/irt/irt_random.h
File src/nonsfi/irt/irt_random.h (right):

https://codereview.chromium.org/686723003/diff/60001/src/nonsfi/irt/irt_rando...
src/nonsfi/irt/irt_random.h:14: /* Initializes the urandom fd by the given |fd|.
*/
On 2014/11/04 01:19:52, Mark Seaborn wrote:
> "by" -> "with".  Please comment that this takes ownership of the given FD.

Done.

https://codereview.chromium.org/686723003/diff/60001/src/untrusted/nacl/nacl....
File src/untrusted/nacl/nacl.scons (right):

https://codereview.chromium.org/686723003/diff/60001/src/untrusted/nacl/nacl....
src/untrusted/nacl/nacl.scons:191: if not env.Bit('posix') and not
env.Bit('nonsfi_nacl'):
On 2014/11/04 01:19:52, Mark Seaborn wrote:
> Should just be "if not env.Bit('nonsfi_nacl')" to match libnacl_sys_private
> below.

This is the condition for the nonsfi nacl_random_private lib, checked in
src/nonsfi/irt/build.scons.
Please let me keep them consistent.

BTW, IIUC, posix bit seems always on if nonsfi_nacl bit is set, right? If so
I'll clean them up.

[Mark Seaborn, 2014-11-05 19:01:16]

LGTM

https://codereview.chromium.org/686723003/diff/60001/src/untrusted/nacl/nacl....
File src/untrusted/nacl/nacl.scons (right):

https://codereview.chromium.org/686723003/diff/60001/src/untrusted/nacl/nacl....
src/untrusted/nacl/nacl.scons:191: if not env.Bit('posix') and not
env.Bit('nonsfi_nacl'):
On 2014/11/04 11:16:59, hidehiko wrote:
> On 2014/11/04 01:19:52, Mark Seaborn wrote:
> > Should just be "if not env.Bit('nonsfi_nacl')" to match libnacl_sys_private
> > below.
> 
> This is the condition for the nonsfi nacl_random_private lib, checked in
> src/nonsfi/irt/build.scons.
> Please let me keep them consistent.
> 
> BTW, IIUC, posix bit seems always on if nonsfi_nacl bit is set, right? If so
> I'll clean them up.

I still think you should remove the env.Bit('posix') check.

'posix' is will either be always-true or always-false in this file, so there's
no reason to check it.  (I think it's always-false.)

The only reason that src/nonsfi/irt/build.scons checks 'posix' is that this file
is used in *two* contexts by SConstruct:
 * building with a NaCl toolchain
 * building with a host-OS toolchain
and we want to exclude building it on Windows.

(That double usage is unusual, BTW.  Normally we have separate build.scons and
nacl.scons files for those cases.)

But src/untrusted/nacl/nacl.scons is only ever built with a NaCl toolchain,
never with a host-OS toolchain.

https://codereview.chromium.org/686723003/diff/140001/src/nonsfi/irt/irt_rand...
File src/nonsfi/irt/irt_random.h (right):

https://codereview.chromium.org/686723003/diff/140001/src/nonsfi/irt/irt_rand...
src/nonsfi/irt/irt_random.h:15: * Initializes the urandom fd with the given
|fd|. This function takes the
Nit: "takes ownership" (no "the")

[hidehiko, 2014-11-06 13:22:24]

Thank you for review. Submitting...

https://codereview.chromium.org/686723003/diff/60001/src/untrusted/nacl/nacl....
File src/untrusted/nacl/nacl.scons (right):

https://codereview.chromium.org/686723003/diff/60001/src/untrusted/nacl/nacl....
src/untrusted/nacl/nacl.scons:191: if not env.Bit('posix') and not
env.Bit('nonsfi_nacl'):
On 2014/11/05 19:01:15, Mark Seaborn wrote:
> On 2014/11/04 11:16:59, hidehiko wrote:
> > On 2014/11/04 01:19:52, Mark Seaborn wrote:
> > > Should just be "if not env.Bit('nonsfi_nacl')" to match
libnacl_sys_private
> > > below.
> > 
> > This is the condition for the nonsfi nacl_random_private lib, checked in
> > src/nonsfi/irt/build.scons.
> > Please let me keep them consistent.
> > 
> > BTW, IIUC, posix bit seems always on if nonsfi_nacl bit is set, right? If so
> > I'll clean them up.
> 
> I still think you should remove the env.Bit('posix') check.
> 
> 'posix' is will either be always-true or always-false in this file, so there's
> no reason to check it.  (I think it's always-false.)
> 
> The only reason that src/nonsfi/irt/build.scons checks 'posix' is that this
file
> is used in *two* contexts by SConstruct:
>  * building with a NaCl toolchain
>  * building with a host-OS toolchain
> and we want to exclude building it on Windows.
> 
> (That double usage is unusual, BTW.  Normally we have separate build.scons and
> nacl.scons files for those cases.)
> 
> But src/untrusted/nacl/nacl.scons is only ever built with a NaCl toolchain,
> never with a host-OS toolchain.

I see. Thank you for clarification. Removed "posix".

https://codereview.chromium.org/686723003/diff/140001/src/nonsfi/irt/irt_rand...
File src/nonsfi/irt/irt_random.h (right):

https://codereview.chromium.org/686723003/diff/140001/src/nonsfi/irt/irt_rand...
src/nonsfi/irt/irt_random.h:15: * Initializes the urandom fd with the given
|fd|. This function takes the
On 2014/11/05 19:01:16, Mark Seaborn wrote:
> Nit: "takes ownership" (no "the")

Done.

[hidehiko, 2014-11-06 13:22:44]

The CQ bit was checked by hidehiko@chromium.org

[commit-bot: I haz the power, 2014-11-06 13:23:23]

CQ is trying da patch. Follow status at
 https://chromium-cq-status.appspot.com/patch-status/686723003/160001

[commit-bot: I haz the power, 2014-11-06 13:23:27]

The CQ bit was unchecked by commit-bot@chromium.org

[commit-bot: I haz the power, 2014-11-06 13:23:29]

Failed to apply patch for src/nonsfi/irt/irt_random.h:
While running patch -p1 --forward --force --no-backup-if-mismatch;
  A         src/nonsfi/irt/irt_random.h
  Copied src/nonsfi/linux/irt_exception_handling.h ->
src/nonsfi/irt/irt_random.h
  patching file src/nonsfi/irt/irt_random.h
  Hunk #1 FAILED at 4.
  1 out of 1 hunk FAILED -- saving rejects to file
src/nonsfi/irt/irt_random.h.rej

Patch:   NR 
src/nonsfi/linux/irt_exception_handling.h->src/nonsfi/irt/irt_random.h
Index: src/nonsfi/irt/irt_random.h
diff --git a/src/nonsfi/linux/irt_exception_handling.h
b/src/nonsfi/irt/irt_random.h
similarity index 52%
copy from src/nonsfi/linux/irt_exception_handling.h
copy to src/nonsfi/irt/irt_random.h
index
4a69c1344274ae9fffbeb94fb1091d8ec85dfe79..93dc5696d69a02d05a504285bccc70b2f464d36b
100644
--- a/src/nonsfi/linux/irt_exception_handling.h
+++ b/src/nonsfi/irt/irt_random.h
@@ -4,15 +4,18 @@
  * found in the LICENSE file.
  */
 
-#ifndef NATIVE_CLIENT_SRC_NONSFI_IRT_IRT_EXCEPTION_HANDLING_H_
-#define NATIVE_CLIENT_SRC_NONSFI_IRT_IRT_EXCEPTION_HANDLING_H_ 1
+#ifndef NATIVE_CLIENT_SRC_NONSFI_IRT_IRT_RANDOM_H_
+#define NATIVE_CLIENT_SRC_NONSFI_IRT_IRT_RANDOM_H_ 1
 
 #include "native_client/src/include/nacl_base.h"
 
 EXTERN_C_BEGIN
 
-/* Initialize signal handler at startup before entering sandbox. */
-void nonsfi_initialize_signal_handler(void);
+/*
+ * Initializes the urandom fd with the given |fd|. This function takes
+ * ownership of the given |fd|.
+ */
+void nonsfi_set_urandom_fd(int fd);
 
 EXTERN_C_END

[hidehiko, 2014-11-06 13:58:11]

The CQ bit was checked by hidehiko@chromium.org

[commit-bot: I haz the power, 2014-11-06 13:58:42]

CQ is trying da patch. Follow status at
 https://chromium-cq-status.appspot.com/patch-status/686723003/180001

[commit-bot: I haz the power, 2014-11-06 16:50:26]

Committed patchset #6 (id:180001) as 14056

[Mark Seaborn, 2014-11-06 17:05:32]

https://codereview.chromium.org/686723003/diff/180001/src/nonsfi/irt/irt_rand...
File src/nonsfi/irt/irt_random.h (right):

https://codereview.chromium.org/686723003/diff/180001/src/nonsfi/irt/irt_rand...
src/nonsfi/irt/irt_random.h:18: void nonsfi_set_urandom_fd(int fd);
It occurs to me that if you want to call this from the Chromium side, then
you'll need to declare it in a header in src/public/.

[hidehiko, 2014-11-06 19:53:15]

https://codereview.chromium.org/686723003/diff/180001/src/nonsfi/irt/irt_rand...
File src/nonsfi/irt/irt_random.h (right):

https://codereview.chromium.org/686723003/diff/180001/src/nonsfi/irt/irt_rand...
src/nonsfi/irt/irt_random.h:18: void nonsfi_set_urandom_fd(int fd);
On 2014/11/06 17:05:32, Mark Seaborn wrote:
> It occurs to me that if you want to call this from the Chromium side, then
> you'll need to declare it in a header in src/public/.

Hmm, then should we move src/nonsfi/linux/irt_exception_handling.h, which also
needs to be referred from Chromium side?

[Mark Seaborn, 2014-11-07 02:46:49]

On 6 November 2014 11:53, <hidehiko@chromium.org> wrote:

>
> https://codereview.chromium.org/686723003/diff/180001/src/
> nonsfi/irt/irt_random.h
> File src/nonsfi/irt/irt_random.h (right):
>
> https://codereview.chromium.org/686723003/diff/180001/src/
> nonsfi/irt/irt_random.h#newcode18
> src/nonsfi/irt/irt_random.h:18: void nonsfi_set_urandom_fd(int fd);
> On 2014/11/06 17:05:32, Mark Seaborn wrote:
>
>> It occurs to me that if you want to call this from the Chromium side, then
>
>  you'll need to declare it in a header in src/public/.
>>
>
> Hmm, then should we move src/nonsfi/linux/irt_exception_handling.h,
> which also needs to be referred from Chromium side?


Yes, that one too.  BTW, feel free to send out for review your
Chromium-side change that uses these headers, before you move the headers
to src/public/.  It would be useful to start the review even if some of the
details are going to be tweaked before it's committed.

Cheers,
Mark

-- 
You received this message because you are subscribed to the Google Groups
"Native-Client-Reviews" group.
To unsubscribe from this group and stop receiving emails from it, send an email
to native-client-reviews+unsubscribe@googlegroups.com.
To post to this group, send email to native-client-reviews@googlegroups.com.
Visit this group at http://groups.google.com/group/native-client-reviews.
For more options, visit https://groups.google.com/d/optout.