[NaCl SDK] Map active fds to absolute paths.

native_client_sdk/src/libraries/nacl_io/kernel_proxy.cc

 // Copyright (c) 2012 The Chromium Authors. All rights reserved.
 // Use of this source code is governed by a BSD-style license that can be
 // found in the LICENSE file.
 
 #include "nacl_io/kernel_proxy.h"
 
 #include <assert.h>
 #include <errno.h>
 #include <fcntl.h>
 #include <limits.h>
@@ skipping 154 matching lines @@
     }
   }
 
   ScopedKernelHandle handle(new KernelHandle(fs, node));
   error = handle->Init(O_RDONLY);
   if (error) {
     errno = error;
     return -1;
   }
 
-  return AllocateFD(handle);
+  int fd = AllocateFD(handle);
+  fd_paths_[fd] = GetAbsParts(path).Join();

[binji, 2014/01/06 19:32:04]

this probably should be handled in AllocateFD. fd_paths_ should be protected by
a lock (maybe handle_lock_?), so you'll likely need to add additional parameters
to AllocateFD (or create some new functions that take path/FD to copy path
from).

Hope this makes sense. :)

[Matthew Turk, 2014/01/07 20:33:11]

It does -- and I've gone through and put path as an argument to AllocateFD in
the updated cl.

I did have one question, which was how to handle the potential race conditions
for relative paths being supplied to open() and the CWD changing between the
call to open() and a resolution of the path to an absolute path.

One possible solution is to perform the absolute path resolution inside
KernelProxy (where it happens now) and ensure that the path supplied to
AllocateFD is normalized and absolute, which would result in two sequential
locks (first cwd_ during path normalization and then handle_lock_).  I think
this only applies to open_resource() and open(), as in the new cl they're the
only ones that won't be handling a path that has already been normalized.  Or am
I worrying too much about this?

+  return fd;
 }
 
 int KernelProxy::open(const char* path, int open_flags) {
   ScopedFilesystem fs;
   ScopedNode node;
 
   Error error = AcquireFsAndNode(path, open_flags, &fs, &node);
   if (error) {
     errno = error;
     return -1;
   }
 
   ScopedKernelHandle handle(new KernelHandle(fs, node));
   error = handle->Init(open_flags);
   if (error) {
     errno = error;
     return -1;
   }
 
-  return AllocateFD(handle);
+  int fd = AllocateFD(handle);
+  fd_paths_[fd] = GetAbsParts(path).Join();
+  return fd;
 }
 
 int KernelProxy::pipe(int pipefds[2]) {
   PipeNode* pipe = new PipeNode(stream_mount_.get());
   ScopedNode node(pipe);
 
   if (pipe->Init(O_RDWR) == 0) {
     ScopedKernelHandle handle0(new KernelHandle(stream_mount_, node));
     ScopedKernelHandle handle1(new KernelHandle(stream_mount_, node));
 
@@ skipping 15 matching lines @@
 int KernelProxy::close(int fd) {
   ScopedKernelHandle handle;
   Error error = AcquireHandle(fd, &handle);
   if (error) {
     errno = error;
     return -1;
   }
 
   // Remove the FD from the process open file descriptor map
   FreeFD(fd);
+  // Remove the FD from the map of fd to absolute path
+  fd_paths_.erase(fd);

[Sam Clegg, 2014/01/06 18:52:50]

Would this be better done as part of FreeFD()?

[binji, 2014/01/06 19:32:04]

agreed.

   return 0;
 }
 
 int KernelProxy::dup(int oldfd) {
   ScopedKernelHandle handle;
   Error error = AcquireHandle(oldfd, &handle);
   if (error) {
     errno = error;
     return -1;
   }
 
-  return AllocateFD(handle);
+  int fd = AllocateFD(handle);
+  fd_paths_[fd] = fd_paths_[oldfd];
+  return fd;
 }
 
 int KernelProxy::dup2(int oldfd, int newfd) {
   // If it's the same file handle, just return
   if (oldfd == newfd)
     return newfd;
 
   ScopedKernelHandle old_handle;
   Error error = AcquireHandle(oldfd, &old_handle);
   if (error) {
@@ skipping 254 matching lines @@
 
   int cnt = 0;
   error = handle->GetDents(static_cast<dirent*>(buf), count, &cnt);
   if (error)
     errno = error;
 
   return cnt;
 }
 
 int KernelProxy::fchdir(int fd) {
-  errno = ENOSYS;
-  return -1;
+  ScopedKernelHandle handle;
+  Error error = AcquireHandle(fd, &handle);
+  if (error) {
+    errno = error;
+    return -1;
+  }
+
+  if (!handle->node()->IsaDir()) {
+    errno = ENOTDIR;
+    return -1;
+  }
+
+  FdPathMap_t::iterator iter = fd_paths_.find(fd);
+  if (iter == fd_paths_.end()) {
+    errno = EBADF;
+    return -1;
+  }
+
+  error = SetCWD(iter->second);
+  if (error) {
+    // errno set by SetCWD

[Sam Clegg, 2014/01/06 18:52:50]

Are you sure?  I don't see SetCWD setting errno.

[binji, 2014/01/06 19:32:04]

Only the KernelProxy sets errno. KernelObject::SetCWD returns the error.

+    return -1;
+  }
+  return 0;
 }
 
 int KernelProxy::ftruncate(int fd, off_t length) {
   ScopedKernelHandle handle;
   Error error = AcquireHandle(fd, &handle);
   if (error) {
     errno = error;
     return -1;
   }
 
@@ skipping 1072 matching lines @@
     errno = ENOTSOCK;
     return -1;
   }
 
   return 0;
 }
 
 #endif  // PROVIDES_SOCKET_API
 
 }  // namespace_nacl_io