Plumbing though mode parameter to open, since fusefs can make use of it.

Plumbing though mode parameter to open, since fusefs can make use of it.

Hooking it up where it makes sense in other mounts.

BUG=https://code.google.com/p/chromium/issues/detail?id=413397
TEST=None
R=binji@chromium.org

Committed: https://crrev.com/4af11334ed84a0280f0b0d2eedb2efd748f20b72
Cr-Commit-Position: refs/heads/master@{#294561}

[bradn, 2014-09-11 21:57:19]

Wait forgot the tests...

[Sam Clegg, 2014-09-11 22:22:49]

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

[Sam Clegg, 2014-09-11 22:22:50]

Ben and I have been chatting about the approach to use here.

I'm afraid our conclusion was to do it a bit differently.

How about this:

1. Leave signatures for FileSystem::Open and Node::Init alone.
2. Have the kernel proxy filter calls to ::open and either call
FileSystem::Creat or FileSystem::Open accordingly.

Init() is used a lot (whenever a node is initialized) so having the extra mode
bits there doesn't make much sense.

Sorry to push back on a fiddly change like this .. happy to step in and help
with alternative approach if you'd like.

[bradn, 2014-09-11 22:29:49]

That sounds much better.
Let me give it a go and I'll cry uncle if his something I don't grok.

[Sam Clegg, 2014-09-11 23:47:09]

Much better!

lgtm, pending any comments from ben.

https://codereview.chromium.org/565763002/diff/20001/native_client_sdk/src/li...
File native_client_sdk/src/libraries/nacl_io/kernel_proxy.cc (right):

https://codereview.chromium.org/565763002/diff/20001/native_client_sdk/src/li...
native_client_sdk/src/libraries/nacl_io/kernel_proxy.cc:202: int
KernelProxy::open(const char* path, int open_flags, mode_t mode_flags) {
Just call this mode?

https://codereview.chromium.org/565763002/diff/20001/native_client_sdk/src/li...
File native_client_sdk/src/libraries/nacl_io/kernel_wrap_bionic.cc (right):

https://codereview.chromium.org/565763002/diff/20001/native_client_sdk/src/li...
native_client_sdk/src/libraries/nacl_io/kernel_wrap_bionic.cc:268: *newfd =
ki_open(pathname, oflag, cmode);
Just call this mode.. here and below?

https://codereview.chromium.org/565763002/diff/20001/native_client_sdk/src/li...
File native_client_sdk/src/libraries/nacl_io/memfs/mem_fs.cc (right):

https://codereview.chromium.org/565763002/diff/20001/native_client_sdk/src/li...
native_client_sdk/src/libraries/nacl_io/memfs/mem_fs.cc:99: 
SetMode()?

[binji, 2014-09-11 23:51:41]

lgtm w/ nits

https://codereview.chromium.org/565763002/diff/20001/native_client_sdk/src/li...
File native_client_sdk/src/libraries/nacl_io/filesystem.h (right):

https://codereview.chromium.org/565763002/diff/20001/native_client_sdk/src/li...
native_client_sdk/src/libraries/nacl_io/filesystem.h:76: virtual Error
Open(const Path& path,
make this non-virtual, since it is just a helper function now?

https://codereview.chromium.org/565763002/diff/20001/native_client_sdk/src/li...
File native_client_sdk/src/libraries/nacl_io/kernel_proxy.cc (right):

https://codereview.chromium.org/565763002/diff/20001/native_client_sdk/src/li...
native_client_sdk/src/libraries/nacl_io/kernel_proxy.cc:107: fd =
open("/dev/stdin", O_RDONLY, 0777);
0 for these, since they are not being created?

https://codereview.chromium.org/565763002/diff/20001/native_client_sdk/src/te...
File native_client_sdk/src/tests/nacl_io_test/fuse_fs_test.cc (right):

https://codereview.chromium.org/565763002/diff/20001/native_client_sdk/src/te...
native_client_sdk/src/tests/nacl_io_test/fuse_fs_test.cc:351: fd =
ki_open("/hello", O_RDONLY, 0777);
0 instead of 0777?

[binji, 2014-09-11 23:52:04]

Oh, and what about tests...? :)

[bradn, 2014-09-12 06:17:03]

Added some tests.

https://codereview.chromium.org/565763002/diff/20001/native_client_sdk/src/li...
File native_client_sdk/src/libraries/nacl_io/filesystem.h (right):

https://codereview.chromium.org/565763002/diff/20001/native_client_sdk/src/li...
native_client_sdk/src/libraries/nacl_io/filesystem.h:76: virtual Error
Open(const Path& path,
On 2014/09/11 23:51:40, binji wrote:
> make this non-virtual, since it is just a helper function now?

Done.

https://codereview.chromium.org/565763002/diff/20001/native_client_sdk/src/li...
File native_client_sdk/src/libraries/nacl_io/kernel_proxy.cc (right):

https://codereview.chromium.org/565763002/diff/20001/native_client_sdk/src/li...
native_client_sdk/src/libraries/nacl_io/kernel_proxy.cc:107: fd =
open("/dev/stdin", O_RDONLY, 0777);
On 2014/09/11 23:51:40, binji wrote:
> 0 for these, since they are not being created?

Done.

https://codereview.chromium.org/565763002/diff/20001/native_client_sdk/src/li...
native_client_sdk/src/libraries/nacl_io/kernel_proxy.cc:202: int
KernelProxy::open(const char* path, int open_flags, mode_t mode_flags) {
On 2014/09/11 23:47:08, Sam Clegg wrote:
> Just call this mode?

Done.

https://codereview.chromium.org/565763002/diff/20001/native_client_sdk/src/li...
File native_client_sdk/src/libraries/nacl_io/kernel_wrap_bionic.cc (right):

https://codereview.chromium.org/565763002/diff/20001/native_client_sdk/src/li...
native_client_sdk/src/libraries/nacl_io/kernel_wrap_bionic.cc:268: *newfd =
ki_open(pathname, oflag, cmode);
On 2014/09/11 23:47:08, Sam Clegg wrote:
> Just call this mode.. here and below?

Done.

https://codereview.chromium.org/565763002/diff/20001/native_client_sdk/src/li...
File native_client_sdk/src/libraries/nacl_io/memfs/mem_fs.cc (right):

https://codereview.chromium.org/565763002/diff/20001/native_client_sdk/src/li...
native_client_sdk/src/libraries/nacl_io/memfs/mem_fs.cc:99: 
On 2014/09/11 23:47:08, Sam Clegg wrote:
> SetMode()?

Done.

https://codereview.chromium.org/565763002/diff/20001/native_client_sdk/src/te...
File native_client_sdk/src/tests/nacl_io_test/fuse_fs_test.cc (right):

https://codereview.chromium.org/565763002/diff/20001/native_client_sdk/src/te...
native_client_sdk/src/tests/nacl_io_test/fuse_fs_test.cc:351: fd =
ki_open("/hello", O_RDONLY, 0777);
On 2014/09/11 23:51:41, binji wrote:
> 0 instead of 0777?

Done.

[bradn, 2014-09-12 07:30:34]

The CQ bit was checked by bradnelson@google.com

[commit-bot: I haz the power, 2014-09-12 07:30:50]

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

[commit-bot: I haz the power, 2014-09-12 09:38:24]

Committed patchset #4 (id:60001) as cd06831942e242c0e66efffe360632a978de7f52

[commit-bot: I haz the power, 2014-09-12 09:40:43]

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