Clean up locking.

native_client_sdk/src/libraries/nacl_io/kernel_object.h

Index: native_client_sdk/src/libraries/nacl_io/kernel_object.h
diff --git a/native_client_sdk/src/libraries/nacl_io/kernel_object.h b/native_client_sdk/src/libraries/nacl_io/kernel_object.h
index 6dc695999a9b68c919df2e74f43f3d1a40bca4f6..a49550e1f065bcaa5322a2c7962346b67d50fda5 100644
--- a/native_client_sdk/src/libraries/nacl_io/kernel_object.h
+++ b/native_client_sdk/src/libraries/nacl_io/kernel_object.h
@@ -14,13 +14,18 @@
 #include "nacl_io/error.h"
 #include "nacl_io/kernel_handle.h"
 #include "nacl_io/mount.h"
+#include "nacl_io/mount_node.h"
 #include "nacl_io/path.h"
 
 
-// KernelObject provides basic functionality for threadsafe
-// access to kernel objects such as file descriptors and
-// file handles.  It also provides access to the CWD for
-// path resolution.
+class PepperInterface;

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

this isn't used in this file, remove

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

Done.

+
+
+// KernelObject provides basic functionality for threadsafe access to kernel
+// objects such as the CWD, mount points, file descriptors and file handles.
+// All Kernel locks are private to ensure the lock order.
+//
+// All calls are assumed to be a relative path.
 class KernelObject {
  public:
   typedef std::vector<ScopedKernelHandle> HandleMap_t;
@@ -29,11 +34,26 @@ class KernelObject {
   KernelObject();
   virtual ~KernelObject();
 
-  // Find the mount for the given path, and acquires it.
-  // Assumes |out_mount| and |out_path| are non-NULL.
-  Error AcquireMountAndPath(const std::string& relpath,
+  // Attach the given Mount object at the specified path.
+  Error MountAtPath(const ScopedMount& mnt, const std::string& path);

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

MountAtPath is a bit confusing because mount can be interpreted as a noun or
verb. Maybe AttachMountAtPath?

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

Done.

+
+  // Unmap the Mount object from the specified path and release it.
+  Error UnmountPath(const std::string& path);

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

Maybe rename this to DetachMountAtPath to match above?

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

Done.

+
+  // Find the mount for the given path, and acquires it and return a
+  // path relative to the mount.
+  // Assumes |out_mount| and |rel_path| are non-NULL.
+  Error AcquireMountAndRelPath(const std::string& path,
+                               ScopedMount* out_mount,
+                               Path* rel_path);
+
+  // Find the mount and node for the given path, acquiring/creating it add

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

remove "add"

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

Done.

+  // as specified by the |oflags|.
+  // Assumes |out_mount| and |out_node| are non-NULL.
+  Error AcquireMountAndNode(const std::string& path,
+                            int oflags,
                             ScopedMount* out_mount,
-                            Path* out_path);
+                            ScopedMountNode* out_node);
 
   // Convert from FD to KernelHandle, and acquire the handle.
   // Assumes |out_handle| is non-NULL.
@@ -48,20 +68,25 @@ class KernelObject {
   void FreeAndReassignFD(int fd, const ScopedKernelHandle& handle);
   void FreeFD(int fd);
 
- protected:
-  Path GetAbsPathLocked(const std::string& path);
+  // Returns an absolute path for the given relative path
+  std::string GetAbsPath(const std::string& path);
+  std::string GetCWD();
+  Error SetCWD(const std::string& path);
 
-  std::vector<int> free_fds_;
+private:
   std::string cwd_;
-
+  std::vector<int> free_fds_;
   HandleMap_t handle_map_;
   MountMap_t mounts_;
 
-  // Kernel lock protects kernel wide resources such as the mount table...
-  pthread_mutex_t kernel_lock_;
+  // Lock to protect mapping from FD to KernelHandle

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

I'd prefer specifying the member variable that is protected here instead:

"Lock to protect handle_map_."

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

Done.

+  pthread_mutex_t handle_lock_;
+
+  // Lock to protect mapping from path to Mount

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

ditto

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

Done.

+  pthread_mutex_t mount_lock_;
 
-  // Process lock protects process wide resources such as CWD, file handles...
-  pthread_mutex_t process_lock_;
+  // Lock to protect cwd
+  pthread_mutex_t cwd_lock_;
 
   DISALLOW_COPY_AND_ASSIGN(KernelObject);
 };