Clean up locking.

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 8d5655ab59945c927ea65be2633d1b825051bcaf..385bfa8b3503019d1dec7a7dd4701ecd1d8a9d53 100644
--- a/native_client_sdk/src/libraries/nacl_io/kernel_proxy.cc
+++ b/native_client_sdk/src/libraries/nacl_io/kernel_proxy.cc
@@ -37,7 +37,8 @@
 #define USR_ID 1002
 #define GRP_ID 1003
 
-KernelProxy::KernelProxy() : dev_(0), ppapi_(NULL) {}
+KernelProxy::KernelProxy() : dev_(0), ppapi_(NULL) {
+}
 
 KernelProxy::~KernelProxy() {
   // Clean up the MountFactories.
@@ -52,7 +53,6 @@ KernelProxy::~KernelProxy() {
 
 void KernelProxy::Init(PepperInterface* ppapi) {
   ppapi_ = ppapi;
-  cwd_ = "/";
   dev_ = 1;
 
   factories_["memfs"] = new TypedMountFactory<MountMem>;
@@ -75,17 +75,10 @@ void KernelProxy::Init(PepperInterface* ppapi) {
 }
 

[binji, 2013/07/10 21:24:34]

move open_resource up here to match header order?

[noelallen1, 2013/07/10 22:11:07]

Done.

 int KernelProxy::open(const char* path, int oflags) {
-  Path rel;
-
   ScopedMount mnt;
-  Error error = AcquireMountAndPath(path, &mnt, &rel);
-  if (error) {
-    errno = error;
-    return -1;
-  }
-
   ScopedMountNode node;
-  error = mnt->Open(rel, oflags, &node);
+
+  Error error = AcquireMountAndNode(path, oflags, &mnt, &node);
   if (error) {
     errno = error;
     return -1;
@@ -141,19 +134,30 @@ int KernelProxy::dup2(int oldfd, int newfd) {
   return newfd;
 }
 
+int KernelProxy::chdir(const char* path) {
+  Error error = SetCWD(path);
+  if (error) {
+    errno = error;
+    return -1;
+  }
+  return 0;
+}
+
 char* KernelProxy::getcwd(char* buf, size_t size) {
-  AutoLock lock(&process_lock_);
+  std::string cwd = GetCWD();
+
   if (size <= 0) {
     errno = EINVAL;
     return NULL;
   }
+
   // If size is 0, allocate as much as we need.
   if (size == 0) {
-    size = cwd_.size() + 1;
+    size = cwd.size() + 1;
   }
 
   // Verify the buffer is large enough
-  if (size <= cwd_.size()) {
+  if (size <= cwd.size()) {
     errno = ERANGE;
     return NULL;
   }
@@ -163,7 +167,7 @@ char* KernelProxy::getcwd(char* buf, size_t size) {
     buf = static_cast<char*>(malloc(size));
   }
 
-  strcpy(buf, cwd_.c_str());
+  strcpy(buf, cwd.c_str());
   return buf;
 }
 
@@ -197,14 +201,15 @@ int KernelProxy::lchown(const char* path, uid_t owner, gid_t group) {
   return 0;
 }
 
-int KernelProxy::utime(const char* filename, const struct utimbuf* times) {
+int KernelProxy::utime(const char* path, const struct utimbuf* times) {

[binji, 2013/07/10 21:24:34]

"filename" matches the linux man page... but if you want to change it here,
change it in the header too

[noelallen1, 2013/07/10 22:11:07]

Done.

   return 0;
 }
 
 int KernelProxy::mkdir(const char* path, mode_t mode) {
   ScopedMount mnt;
   Path rel;
-  Error error = AcquireMountAndPath(path, &mnt, &rel);
+
+  Error error = AcquireMountAndRelPath(path, &mnt, &rel);
   if (error) {
     errno = error;
     return -1;
@@ -222,7 +227,8 @@ int KernelProxy::mkdir(const char* path, mode_t mode) {
 int KernelProxy::rmdir(const char* path) {
   ScopedMount mnt;
   Path rel;
-  Error error = AcquireMountAndPath(path, &mnt, &rel);
+
+  Error error = AcquireMountAndRelPath(path, &mnt, &rel);
   if (error) {
     errno = error;
     return -1;
@@ -247,42 +253,12 @@ int KernelProxy::stat(const char* path, struct stat* buf) {
   return result;
 }
 
-int KernelProxy::chdir(const char* path) {
-  struct stat statbuf;
-  if (stat(path, &statbuf) == -1)
-    return -1;
-
-  bool is_dir = (statbuf.st_mode & S_IFDIR) != 0;
-  if (!is_dir) {
-    errno = ENOTDIR;
-    return -1;
-  }
-
-  AutoLock lock(&process_lock_);
-  cwd_ = GetAbsPathLocked(path).Join();
-  return 0;
-}
 
 int KernelProxy::mount(const char* source,
                        const char* target,
                        const char* filesystemtype,
                        unsigned long mountflags,
                        const void* data) {
-  // See if it's already mounted
-  std::string abs_targ;
-
-  // Scope this lock to prevent holding both process and kernel locks
-  {
-    AutoLock lock(&process_lock_);
-    abs_targ = GetAbsPathLocked(target).Join();
-  }
-
-  AutoLock lock(&kernel_lock_);
-  if (mounts_.find(abs_targ) != mounts_.end()) {
-    errno = EBUSY;
-    return -1;
-  }
-
   // Find a factory of that type
   MountFactoryMap_t::iterator factory = factories_.find(filesystemtype);
   if (factory == factories_.end()) {
@@ -290,9 +266,10 @@ int KernelProxy::mount(const char* source,
     return -1;
   }
 
+  // Create a map of settings
   StringMap_t smap;
   smap["SOURCE"] = source;
-  smap["TARGET"] = abs_targ;
+  smap["TARGET"] = target;

[binji, 2013/07/10 21:24:34]

need to make this absolute, right?

[noelallen1, 2013/07/10 22:11:07]

Done.

 
   if (data) {
     char* str = strdup(static_cast<const char*>(data));
@@ -311,8 +288,16 @@ int KernelProxy::mount(const char* source,
     free(str);
   }
 
+  ScopedMount mnt;
   Error error =

[binji, 2013/07/10 21:24:34]

fits on one line now

[noelallen1, 2013/07/10 22:11:07]

Done.

-      factory->second->CreateMount(dev_++, smap, ppapi_, &mounts_[abs_targ]);
+      factory->second->CreateMount(dev_++, smap, ppapi_, &mnt);
+
+  if (error) {
+    errno = error;
+    return -1;
+  }
+
+  error = MountAtPath(mnt, target);
   if (error) {
     errno = error;
     return -1;
@@ -322,28 +307,11 @@ int KernelProxy::mount(const char* source,
 }
 
 int KernelProxy::umount(const char* path) {
-  Path abs_path;
-
-  // Scope this lock to prevent holding both process and kernel locks
-  {
-    AutoLock lock(&process_lock_);
-    abs_path = GetAbsPathLocked(path);
-  }
-
-  AutoLock lock(&kernel_lock_);
-  MountMap_t::iterator it = mounts_.find(abs_path.Join());
-
-  if (mounts_.end() == it) {
-    errno = EINVAL;
-    return -1;
-  }
-
-  if (it->second->RefCount() != 1) {
-    errno = EBUSY;
+  Error error = UnmountPath(path);
+  if (error) {
+    errno = error;
     return -1;
   }
-
-  mounts_.erase(it);
   return 0;
 }
 
@@ -519,7 +487,8 @@ off_t KernelProxy::lseek(int fd, off_t offset, int whence) {
 int KernelProxy::unlink(const char* path) {
   ScopedMount mnt;
   Path rel;
-  Error error = AcquireMountAndPath(path, &mnt, &rel);
+
+  Error error = AcquireMountAndRelPath(path, &mnt, &rel);
   if (error) {
     errno = error;
     return -1;
@@ -537,7 +506,8 @@ int KernelProxy::unlink(const char* path) {
 int KernelProxy::remove(const char* path) {
   ScopedMount mnt;
   Path rel;
-  Error error = AcquireMountAndPath(path, &mnt, &rel);
+
+  Error error = AcquireMountAndRelPath(path, &mnt, &rel);
   if (error) {
     errno = error;
     return -1;
@@ -565,10 +535,10 @@ int KernelProxy::fchmod(int fd, int mode) {
 }
 
 int KernelProxy::access(const char* path, int amode) {
+  ScopedMount mnt;
   Path rel;
 
-  ScopedMount mnt;
-  Error error = AcquireMountAndPath(path, &mnt, &rel);
+  Error error = AcquireMountAndRelPath(path, &mnt, &rel);
   if (error) {
     errno = error;
     return -1;
@@ -659,7 +629,8 @@ int KernelProxy::munmap(void* addr, size_t length) {
 int KernelProxy::open_resource(const char* path) {
   ScopedMount mnt;
   Path rel;
-  Error error = AcquireMountAndPath(path, &mnt, &rel);
+
+  Error error = AcquireMountAndRelPath(path, &mnt, &rel);
   if (error) {
     errno = error;
     return -1;