[NaCl SDK] Map active fds to absolute paths.

native_client_sdk/src/libraries/nacl_io/kernel_proxy.cc

Index: native_client_sdk/src/libraries/nacl_io/kernel_proxy.cc
diff --git a/native_client_sdk/src/libraries/nacl_io/kernel_proxy.cc b/native_client_sdk/src/libraries/nacl_io/kernel_proxy.cc
index 672f51c30825aa1c9a8b4febf07f1047128e7b0c..3badd25efde968446437b8caeed1fd1a65d88c8a 100644
--- a/native_client_sdk/src/libraries/nacl_io/kernel_proxy.cc
+++ b/native_client_sdk/src/libraries/nacl_io/kernel_proxy.cc
@@ -172,7 +172,9 @@ int KernelProxy::open_resource(const char* path) {
     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) {
@@ -192,7 +194,9 @@ int KernelProxy::open(const char* path, int open_flags) {
     return -1;
   }
 
-  return AllocateFD(handle);
+  int fd = AllocateFD(handle);
+  fd_paths_[fd] = GetAbsParts(path).Join();
+  return fd;
 }
 
 int KernelProxy::pipe(int pipefds[2]) {
@@ -228,6 +232,8 @@ int KernelProxy::close(int fd) {
 
   // 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;
 }
 
@@ -239,7 +245,9 @@ int KernelProxy::dup(int oldfd) {
     return -1;
   }
 
-  return AllocateFD(handle);
+  int fd = AllocateFD(handle);
+  fd_paths_[fd] = fd_paths_[oldfd];
+  return fd;
 }
 
 int KernelProxy::dup2(int oldfd, int newfd) {
@@ -514,8 +522,30 @@ int KernelProxy::getdents(int fd, void* buf, unsigned int count) {
 }
 
 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) {