Files: Add basic implementation of Directory and some basic tests.

services/files/directory_impl.cc

Index: services/files/directory_impl.cc
diff --git a/services/files/directory_impl.cc b/services/files/directory_impl.cc
index c346ce692bfaa7b76092f10eb44d9ea475490e4a..5e2682eb4a97174ffc6ec93716923372caa9d657 100644
--- a/services/files/directory_impl.cc
+++ b/services/files/directory_impl.cc
@@ -4,6 +4,7 @@
 
 #include "services/files/directory_impl.h"
 
+#include <dirent.h>
 #include <errno.h>
 #include <fcntl.h>
 #include <stdio.h>
@@ -12,13 +13,13 @@
 #include <time.h>
 #include <unistd.h>
 
-#include "base/files/file_path.h"
 #include "base/files/scoped_temp_dir.h"
 #include "base/logging.h"
 #include "base/memory/scoped_ptr.h"
 #include "base/posix/eintr_wrapper.h"
 #include "build/build_config.h"
 #include "services/files/file_impl.h"
+#include "services/files/futimens.h"
 #include "services/files/util.h"
 
 namespace mojo {
@@ -26,6 +27,12 @@ namespace files {
 
 namespace {
 
+// Calls |closedir()| on a |DIR*|.
+struct DIRDeleter {
+  void operator()(DIR* dir) const { PCHECK(closedir(dir) == 0); }
+};
+typedef scoped_ptr<DIR, DIRDeleter> ScopedDIR;
+
 Error ValidateOpenFlags(uint32_t open_flags, bool is_directory) {
   // Treat unknown flags as "unimplemented".
   if ((open_flags &
@@ -65,6 +72,27 @@ Error ValidateOpenFlags(uint32_t open_flags, bool is_directory) {
   return ERROR_OK;
 }
 
+Error ValidateDeleteFlags(uint32_t delete_flags) {
+  // Treat unknown flags as "unimplemented".
+  if ((delete_flags &
+       ~(kDeleteFlagFileOnly | kDeleteFlagDirectoryOnly |
+         kDeleteFlagRecursive)))
+    return ERROR_UNIMPLEMENTED;
+
+  // Only one of the three currently-defined flags may be set.
+  if ((delete_flags & kDeleteFlagFileOnly) &&
+      (delete_flags & (kDeleteFlagDirectoryOnly | kDeleteFlagRecursive)))
+    return ERROR_INVALID_ARGUMENT;
+  if ((delete_flags & kDeleteFlagDirectoryOnly) &&
+      (delete_flags & (kDeleteFlagFileOnly | kDeleteFlagRecursive)))
+    return ERROR_INVALID_ARGUMENT;
+  if ((delete_flags & kDeleteFlagRecursive) &&
+      (delete_flags & (kDeleteFlagFileOnly | kDeleteFlagDirectoryOnly)))
+    return ERROR_INVALID_ARGUMENT;
+
+  return ERROR_OK;
+}
+
 }  // namespace
 
 DirectoryImpl::DirectoryImpl(InterfaceRequest<Directory> request,
@@ -81,24 +109,118 @@ DirectoryImpl::~DirectoryImpl() {
 
 void DirectoryImpl::Read(
     const Callback<void(Error, Array<DirectoryEntryPtr>)>& callback) {
-  // TODO(vtl): FIXME sooner
-  NOTIMPLEMENTED();
-  callback.Run(ERROR_UNIMPLEMENTED, Array<DirectoryEntryPtr>());
+  static const size_t kMaxReadCount = 1000;
+
+  DCHECK(dir_fd_.is_valid());
+
+  // |fdopendir()| takes ownership of the FD (giving it to the |DIR| --
+  // |closedir()| will close the FD)), so we need to |dup()| ours.
+  base::ScopedFD fd(dup(dir_fd_.get()));
+  if (!fd.is_valid()) {
+    callback.Run(ErrnoToError(errno), Array<DirectoryEntryPtr>());
+    return;
+  }
+
+  ScopedDIR dir(fdopendir(fd.release()));
+  if (!dir) {
+    callback.Run(ErrnoToError(errno), Array<DirectoryEntryPtr>());
+    return;
+  }
+
+  Array<DirectoryEntryPtr> result(0);
+
+  // Warning: This is not portable (per POSIX.1 -- |buffer| may not be large
+  // enough), but it's fine for Linux.

[qsr, 2015/03/06 13:01:04]

Do you want to put a static assert on OS_ANDROID || OS_LINUX do that we know
when we are not safe anymore?

[viettrungluu, 2015/03/09 17:59:29]

I'll use an #if !defined(...) and #error.

+  struct dirent buffer;
+  for (size_t n = 0;;) {
+    struct dirent* entry = nullptr;
+    if (int error = readdir_r(dir.get(), &buffer, &entry)) {
+      // |error| is effectively an errno (for |readdir_r()|), AFAICT.
+      callback.Run(ErrnoToError(error), Array<DirectoryEntryPtr>());
+      return;
+    }
+
+    if (!entry)
+      break;
+
+    n++;
+    if (n > kMaxReadCount) {
+      LOG(WARNING) << "Directory contents truncated";
+      callback.Run(ERROR_OUT_OF_RANGE, result.Pass());
+      return;
+    }
+
+    DirectoryEntryPtr e = DirectoryEntry::New();
+    switch (entry->d_type) {
+      case DT_DIR:
+        e->type = FILE_TYPE_DIRECTORY;
+        break;
+      case DT_REG:
+        e->type = FILE_TYPE_REGULAR_FILE;
+        break;
+      default:
+        e->type = FILE_TYPE_UNKNOWN;

[qsr, 2015/03/06 13:01:04]

I wonder if we should not just hide all of those entries instead.

[viettrungluu, 2015/03/09 17:59:28]

It's tempting, but then it'd be odd if someone tried to create a file and failed
since something of that name already exists (or tried to remove an
apparently-empty directory and failed).

+        break;
+    }
+    e->name = String(entry->d_name);
+    result.push_back(e.Pass());
+  }
+
+  callback.Run(ERROR_OK, result.Pass());
 }
 
 void DirectoryImpl::Stat(
     const Callback<void(Error, FileInformationPtr)>& callback) {
-  // TODO(vtl): FIXME sooner
-  NOTIMPLEMENTED();
-  callback.Run(ERROR_UNIMPLEMENTED, nullptr);
+  DCHECK(dir_fd_.is_valid());
+
+  // TODO(vtl): The code below is copied from |FileImpl::Stat()|.

[qsr, 2015/03/06 13:01:04]

Could you extract it in a helper function then?

[viettrungluu, 2015/03/09 17:59:29]

Done.

+  struct stat buf;
+  if (fstat(dir_fd_.get(), &buf) != 0) {
+    callback.Run(ErrnoToError(errno), nullptr);
+    return;
+  }
+
+  FileInformationPtr file_info(FileInformation::New());
+  file_info->size = 0;  // Always zero for directories.
+  file_info->atime = Timespec::New();
+  file_info->mtime = Timespec::New();
+#if defined(OS_ANDROID)
+  file_info->atime->seconds = static_cast<int64_t>(buf.st_atime);
+  file_info->atime->nanoseconds = static_cast<int32_t>(buf.st_atime_nsec);
+  file_info->mtime->seconds = static_cast<int64_t>(buf.st_mtime);
+  file_info->mtime->nanoseconds = static_cast<int32_t>(buf.st_mtime_nsec);
+#else
+  file_info->atime->seconds = static_cast<int64_t>(buf.st_atim.tv_sec);
+  file_info->atime->nanoseconds = static_cast<int32_t>(buf.st_atim.tv_nsec);
+  file_info->mtime->seconds = static_cast<int64_t>(buf.st_mtim.tv_sec);
+  file_info->mtime->nanoseconds = static_cast<int32_t>(buf.st_mtim.tv_nsec);
+#endif
+
+  callback.Run(ERROR_OK, file_info.Pass());
 }
 
 void DirectoryImpl::Touch(TimespecOrNowPtr atime,
                           TimespecOrNowPtr mtime,
                           const Callback<void(Error)>& callback) {
-  // TODO(vtl): FIXME sooner
-  NOTIMPLEMENTED();
-  callback.Run(ERROR_UNIMPLEMENTED);
+  DCHECK(dir_fd_.is_valid());
+
+  // TODO(vtl): The code below is copied from |FileImpl::Stat()|.

[qsr, 2015/03/06 13:01:04]

"

[viettrungluu, 2015/03/09 17:59:29]

Done.

+  struct timespec times[2];
+  if (Error error = TimespecOrNowToStandardTimespec(atime.get(), &times[0])) {
+    callback.Run(error);
+    return;
+  }
+  if (Error error = TimespecOrNowToStandardTimespec(mtime.get(), &times[1])) {
+    callback.Run(error);
+    return;
+  }
+
+  if (futimens(dir_fd_.get(), times) != 0) {
+    callback.Run(ErrnoToError(errno));
+    return;
+  }
+
+  callback.Run(ERROR_OK);
 }
 
 // TODO(vtl): Move the implementation to a thread pool.
@@ -151,9 +273,49 @@ void DirectoryImpl::OpenDirectory(const String& path,
                                   InterfaceRequest<Directory> directory,
                                   uint32_t open_flags,
                                   const Callback<void(Error)>& callback) {
-  // TODO(vtl): FIXME sooner
-  NOTIMPLEMENTED();
-  callback.Run(ERROR_UNIMPLEMENTED);
+  DCHECK(!path.is_null());
+  DCHECK(dir_fd_.is_valid());
+
+  if (Error error = IsPathValid(path)) {
+    callback.Run(error);
+    return;
+  }
+  // TODO(vtl): Make sure the path doesn't exit this directory (if appropriate).
+  // TODO(vtl): Maybe allow absolute paths?
+
+  if (Error error = ValidateOpenFlags(open_flags, false)) {
+    callback.Run(error);
+    return;
+  }
+
+  // TODO(vtl): Implement read-only (whatever that means).
+  if (!(open_flags & kOpenFlagWrite)) {
+    callback.Run(ERROR_UNIMPLEMENTED);
+    return;
+  }
+
+  if ((open_flags & kOpenFlagCreate)) {
+    if (mkdirat(dir_fd_.get(), path.get().c_str(), 0700) != 0) {
+      // Allow |EEXIST| if |kOpenFlagExclusive| is set. Note, however, that it

[qsr, 2015/03/06 13:01:04]

is not set?

[viettrungluu, 2015/03/09 17:59:29]

Done.

+      // does not guarantee that |path| is a directory.
+      // TODO(vtl): Hrm, ponder if we should check that |path| is a directory.
+      if (errno != EEXIST || !(open_flags & kOpenFlagExclusive)) {
+        callback.Run(ErrnoToError(errno));
+        return;
+      }
+    }
+  }
+
+  base::ScopedFD new_dir_fd(
+      HANDLE_EINTR(openat(dir_fd_.get(), path.get().c_str(), O_DIRECTORY, 0)));
+  if (!new_dir_fd.is_valid()) {
+    callback.Run(ErrnoToError(errno));
+    return;
+  }
+
+  if (directory.is_pending())
+    new DirectoryImpl(directory.Pass(), new_dir_fd.Pass(), nullptr);
+  callback.Run(ERROR_OK);
 }
 
 void DirectoryImpl::Rename(const String& path,
@@ -185,9 +347,48 @@ void DirectoryImpl::Rename(const String& path,
 void DirectoryImpl::Delete(const String& path,
                            uint32_t delete_flags,
                            const Callback<void(Error)>& callback) {
-  // TODO(vtl): FIXME sooner
-  NOTIMPLEMENTED();
-  callback.Run(ERROR_UNIMPLEMENTED);
+  DCHECK(!path.is_null());
+  // TODO(vtl): Should we check that |path| is nonempty?

[qsr, 2015/03/06 13:01:04]

You would need more than non-empty. foo/.. is not better than ""

We will probably need some notion of resolving path sometime I guess.

[viettrungluu, 2015/03/09 17:59:28]

Errrr, right. I guess this falls under the "see TODOs about |path| in
OpenFile()" below.

+  DCHECK(dir_fd_.is_valid());
+
+  if (Error error = IsPathValid(path)) {
+    callback.Run(error);
+    return;
+  }
+  // TODO(vtl): see TODOs about |path| in OpenFile()
+
+  if (Error error = ValidateDeleteFlags(delete_flags)) {
+    callback.Run(error);
+    return;
+  }
+
+  // TODO(vtl): Recursive not yet supported.
+  if ((delete_flags & kDeleteFlagRecursive)) {
+    callback.Run(ERROR_UNIMPLEMENTED);
+    return;
+  }
+
+  // First try deleting it as a file, unless we're told to do directory-only.

[qsr, 2015/03/06 13:01:04]

You do not use fstatat because of the race?

Also, do we allow to delete file on unknown type? Especially if
kDeleteFlagFileOnly is set?

[viettrungluu, 2015/03/09 17:59:28]

Right.
Hmmm, I guess kDeleteFlagFileOnly is meant to mimic the behavior of unlink() (or
unlinkat() without AT_REMOVEDIR). Possibly, it should be renamed to
|kDeleteFlagNotDirectory|, or something like that (name suggestions welcome).

(Or we could say that anything that's not a directory is a (possibly-special)
"file".)

+  if (!(delete_flags & kDeleteFlagDirectoryOnly)) {
+    if (unlinkat(dir_fd_.get(), path.get().c_str(), 0) == 0) {
+      callback.Run(ERROR_OK);
+      return;
+    }
+
+    // If file-only, don't continue.
+    if ((delete_flags & kDeleteFlagFileOnly)) {
+      callback.Run(ErrnoToError(errno));
+      return;
+    }
+  }
+
+  // Try deleting it as a directory.
+  if (unlinkat(dir_fd_.get(), path.get().c_str(), AT_REMOVEDIR) == 0) {
+    callback.Run(ERROR_OK);
+    return;
+  }
+
+  callback.Run(ErrnoToError(errno));
 }
 
 }  // namespace files