[NaCl SDK] nacl_io: handle null irt function pointers

[NaCl SDK] nacl_io: handle null irt function pointers

Under pnacl the filename-dev irt interface is not
available which means the "real" pointers end up
being NULL.

Without this fix nacl_io will crash on fist call
to open(2) in the passthrough fs (the default root
filesystem!).

Technically this change is only needed for newlib
today but applying to all libc versions since the
availability of dev interfaces is never guaranteed.

Committed: https://crrev.com/b8bb8fcd9db486a59ecd8e16fd580ca99704e4ad
Cr-Commit-Position: refs/heads/master@{#295414}

[Sam Clegg, 2014-09-17 22:55:05]

sbc@chromium.org changed reviewers:
+ binji@chromium.org

[binji, 2014-09-17 23:30:45]

I cross-checked against irt_dev.h and found some other spots, I think.

Otherwise, lgtm

https://codereview.chromium.org/576363002/diff/1/native_client_sdk/src/librar...
File native_client_sdk/src/libraries/nacl_io/kernel_wrap_bionic.cc (right):

https://codereview.chromium.org/576363002/diff/1/native_client_sdk/src/librar...
native_client_sdk/src/libraries/nacl_io/kernel_wrap_bionic.cc:361:
CHECK_REAL(fchdir);
here too?

https://codereview.chromium.org/576363002/diff/1/native_client_sdk/src/librar...
native_client_sdk/src/libraries/nacl_io/kernel_wrap_bionic.cc:366:
CHECK_REAL(fchmod);
here too?

https://codereview.chromium.org/576363002/diff/1/native_client_sdk/src/librar...
native_client_sdk/src/libraries/nacl_io/kernel_wrap_bionic.cc:371:
CHECK_REAL(fdatasync);
here too?

https://codereview.chromium.org/576363002/diff/1/native_client_sdk/src/librar...
native_client_sdk/src/libraries/nacl_io/kernel_wrap_bionic.cc:391: return
REAL(fsync)(fd);
here too?

https://codereview.chromium.org/576363002/diff/1/native_client_sdk/src/librar...
File native_client_sdk/src/libraries/nacl_io/kernel_wrap_glibc.cc (right):

https://codereview.chromium.org/576363002/diff/1/native_client_sdk/src/librar...
native_client_sdk/src/libraries/nacl_io/kernel_wrap_glibc.cc:550:
CHECK_REAL(fstat);
here too?

https://codereview.chromium.org/576363002/diff/1/native_client_sdk/src/librar...
File native_client_sdk/src/libraries/nacl_io/kernel_wrap_newlib.cc (right):

https://codereview.chromium.org/576363002/diff/1/native_client_sdk/src/librar...
native_client_sdk/src/libraries/nacl_io/kernel_wrap_newlib.cc:293:
CHECK_REAL(fstat);
here too?

https://codereview.chromium.org/576363002/diff/1/native_client_sdk/src/librar...
native_client_sdk/src/libraries/nacl_io/kernel_wrap_newlib.cc:314:
CHECK_REAL(seek);
here too?

[Sam Clegg, 2014-09-17 23:35:24]

On 2014/09/17 23:30:45, binji wrote:
> I cross-checked against irt_dev.h and found some other spots, I think.
> 
> Otherwise, lgtm
> 
>
https://codereview.chromium.org/576363002/diff/1/native_client_sdk/src/librar...
> File native_client_sdk/src/libraries/nacl_io/kernel_wrap_bionic.cc (right):
> 
>
https://codereview.chromium.org/576363002/diff/1/native_client_sdk/src/librar...
> native_client_sdk/src/libraries/nacl_io/kernel_wrap_bionic.cc:361:
> CHECK_REAL(fchdir);
> here too?
> 
>
https://codereview.chromium.org/576363002/diff/1/native_client_sdk/src/librar...
> native_client_sdk/src/libraries/nacl_io/kernel_wrap_bionic.cc:366:
> CHECK_REAL(fchmod);
> here too?
> 
>
https://codereview.chromium.org/576363002/diff/1/native_client_sdk/src/librar...
> native_client_sdk/src/libraries/nacl_io/kernel_wrap_bionic.cc:371:
> CHECK_REAL(fdatasync);
> here too?
> 
>
https://codereview.chromium.org/576363002/diff/1/native_client_sdk/src/librar...
> native_client_sdk/src/libraries/nacl_io/kernel_wrap_bionic.cc:391: return
> REAL(fsync)(fd);
> here too?
> 
>
https://codereview.chromium.org/576363002/diff/1/native_client_sdk/src/librar...
> File native_client_sdk/src/libraries/nacl_io/kernel_wrap_glibc.cc (right):
> 
>
https://codereview.chromium.org/576363002/diff/1/native_client_sdk/src/librar...
> native_client_sdk/src/libraries/nacl_io/kernel_wrap_glibc.cc:550:
> CHECK_REAL(fstat);
> here too?
> 
>
https://codereview.chromium.org/576363002/diff/1/native_client_sdk/src/librar...
> File native_client_sdk/src/libraries/nacl_io/kernel_wrap_newlib.cc (right):
> 
>
https://codereview.chromium.org/576363002/diff/1/native_client_sdk/src/librar...
> native_client_sdk/src/libraries/nacl_io/kernel_wrap_newlib.cc:293:
> CHECK_REAL(fstat);
> here too?
> 
>
https://codereview.chromium.org/576363002/diff/1/native_client_sdk/src/librar...
> native_client_sdk/src/libraries/nacl_io/kernel_wrap_newlib.cc:314:
> CHECK_REAL(seek);
> here too?

Maybe we should just do this for all real ptrs?  Are there any where is makes
sense to assert(ptr != NULL) instead?  (write maybe?  but if thats null then all
bets are off because its out lowest level print primitive).

[binji, 2014-09-17 23:37:22]

On 2014/09/17 23:35:24, Sam Clegg wrote:
> On 2014/09/17 23:30:45, binji wrote:
> > I cross-checked against irt_dev.h and found some other spots, I think.
> > 
> > Otherwise, lgtm
> > 
> >
>
https://codereview.chromium.org/576363002/diff/1/native_client_sdk/src/librar...
> > File native_client_sdk/src/libraries/nacl_io/kernel_wrap_bionic.cc (right):
> > 
> >
>
https://codereview.chromium.org/576363002/diff/1/native_client_sdk/src/librar...
> > native_client_sdk/src/libraries/nacl_io/kernel_wrap_bionic.cc:361:
> > CHECK_REAL(fchdir);
> > here too?
> > 
> >
>
https://codereview.chromium.org/576363002/diff/1/native_client_sdk/src/librar...
> > native_client_sdk/src/libraries/nacl_io/kernel_wrap_bionic.cc:366:
> > CHECK_REAL(fchmod);
> > here too?
> > 
> >
>
https://codereview.chromium.org/576363002/diff/1/native_client_sdk/src/librar...
> > native_client_sdk/src/libraries/nacl_io/kernel_wrap_bionic.cc:371:
> > CHECK_REAL(fdatasync);
> > here too?
> > 
> >
>
https://codereview.chromium.org/576363002/diff/1/native_client_sdk/src/librar...
> > native_client_sdk/src/libraries/nacl_io/kernel_wrap_bionic.cc:391: return
> > REAL(fsync)(fd);
> > here too?
> > 
> >
>
https://codereview.chromium.org/576363002/diff/1/native_client_sdk/src/librar...
> > File native_client_sdk/src/libraries/nacl_io/kernel_wrap_glibc.cc (right):
> > 
> >
>
https://codereview.chromium.org/576363002/diff/1/native_client_sdk/src/librar...
> > native_client_sdk/src/libraries/nacl_io/kernel_wrap_glibc.cc:550:
> > CHECK_REAL(fstat);
> > here too?
> > 
> >
>
https://codereview.chromium.org/576363002/diff/1/native_client_sdk/src/librar...
> > File native_client_sdk/src/libraries/nacl_io/kernel_wrap_newlib.cc (right):
> > 
> >
>
https://codereview.chromium.org/576363002/diff/1/native_client_sdk/src/librar...
> > native_client_sdk/src/libraries/nacl_io/kernel_wrap_newlib.cc:293:
> > CHECK_REAL(fstat);
> > here too?
> > 
> >
>
https://codereview.chromium.org/576363002/diff/1/native_client_sdk/src/librar...
> > native_client_sdk/src/libraries/nacl_io/kernel_wrap_newlib.cc:314:
> > CHECK_REAL(seek);
> > here too?
> 
> Maybe we should just do this for all real ptrs?  Are there any where is makes
> sense to assert(ptr != NULL) instead?  (write maybe?  but if thats null then
all
> bets are off because its out lowest level print primitive).

OK, I think you're right.

[Sam Clegg, 2014-09-18 00:04:39]

On 2014/09/17 23:37:22, binji wrote:
> On 2014/09/17 23:35:24, Sam Clegg wrote:
> > On 2014/09/17 23:30:45, binji wrote:
> > > I cross-checked against irt_dev.h and found some other spots, I think.
> > > 
> > > Otherwise, lgtm
> > > 
> > >
> >
>
https://codereview.chromium.org/576363002/diff/1/native_client_sdk/src/librar...
> > > File native_client_sdk/src/libraries/nacl_io/kernel_wrap_bionic.cc
(right):
> > > 
> > >
> >
>
https://codereview.chromium.org/576363002/diff/1/native_client_sdk/src/librar...
> > > native_client_sdk/src/libraries/nacl_io/kernel_wrap_bionic.cc:361:
> > > CHECK_REAL(fchdir);
> > > here too?
> > > 
> > >
> >
>
https://codereview.chromium.org/576363002/diff/1/native_client_sdk/src/librar...
> > > native_client_sdk/src/libraries/nacl_io/kernel_wrap_bionic.cc:366:
> > > CHECK_REAL(fchmod);
> > > here too?
> > > 
> > >
> >
>
https://codereview.chromium.org/576363002/diff/1/native_client_sdk/src/librar...
> > > native_client_sdk/src/libraries/nacl_io/kernel_wrap_bionic.cc:371:
> > > CHECK_REAL(fdatasync);
> > > here too?
> > > 
> > >
> >
>
https://codereview.chromium.org/576363002/diff/1/native_client_sdk/src/librar...
> > > native_client_sdk/src/libraries/nacl_io/kernel_wrap_bionic.cc:391: return
> > > REAL(fsync)(fd);
> > > here too?
> > > 
> > >
> >
>
https://codereview.chromium.org/576363002/diff/1/native_client_sdk/src/librar...
> > > File native_client_sdk/src/libraries/nacl_io/kernel_wrap_glibc.cc (right):
> > > 
> > >
> >
>
https://codereview.chromium.org/576363002/diff/1/native_client_sdk/src/librar...
> > > native_client_sdk/src/libraries/nacl_io/kernel_wrap_glibc.cc:550:
> > > CHECK_REAL(fstat);
> > > here too?
> > > 
> > >
> >
>
https://codereview.chromium.org/576363002/diff/1/native_client_sdk/src/librar...
> > > File native_client_sdk/src/libraries/nacl_io/kernel_wrap_newlib.cc
(right):
> > > 
> > >
> >
>
https://codereview.chromium.org/576363002/diff/1/native_client_sdk/src/librar...
> > > native_client_sdk/src/libraries/nacl_io/kernel_wrap_newlib.cc:293:
> > > CHECK_REAL(fstat);
> > > here too?
> > > 
> > >
> >
>
https://codereview.chromium.org/576363002/diff/1/native_client_sdk/src/librar...
> > > native_client_sdk/src/libraries/nacl_io/kernel_wrap_newlib.cc:314:
> > > CHECK_REAL(seek);
> > > here too?
> > 
> > Maybe we should just do this for all real ptrs?  Are there any where is
makes
> > sense to assert(ptr != NULL) instead?  (write maybe?  but if thats null then
> all
> > bets are off because its out lowest level print primitive).
> 
> OK, I think you're right.

Done.

[Sam Clegg, 2014-09-18 00:35:35]

The CQ bit was checked by sbc@chromium.org

[commit-bot: I haz the power, 2014-09-18 00:36:09]

CQ is trying da patch. Follow status at
 https://chromium-cq-status.appspot.com/patchset/576363002/20001

[commit-bot: I haz the power, 2014-09-18 04:34:12]

Committed patchset #2 (id:20001) as 87e5555fab97025e2d419758d255876fda00b99f

[commit-bot: I haz the power, 2014-09-18 04:34:47]

Patchset 2 (id:??) landed as
https://crrev.com/b8bb8fcd9db486a59ecd8e16fd580ca99704e4ad
Cr-Commit-Position: refs/heads/master@{#295414}