Convert FileSystem::Open() to return Expected<FileUP>

Summary:
This patch converts FileSystem::Open from this prototype:

Status
Open(File &File, const FileSpec &file_spec, ...);

to this one:

llvm::Expected<std::unique_ptr<File>>
Open(const FileSpec &file_spec, ...);

This is beneficial on its own, as llvm::Expected is a more modern
and recommended error type than Status.  It is also a necessary step
towards https://reviews.llvm.org/D67891, and further developments
for lldb_private::File.

Reviewers: JDevlieghere, jasonmolenda, labath

Reviewed By: labath

Subscribers: mgorny, lldb-commits

Tags: #lldb

Differential Revision: https://reviews.llvm.org/D67996

llvm-svn: 373003

3 files changed, 32 insertions, 33 deletions

diff --git a/lldb/source/Host/common/FileCache.cpp b/lldb/source/Host/common/FileCache.cpp
index 4bd3efda7fb..0a7197e55b0 100644
--- a/lldb/source/Host/common/FileCache.cpp
+++ b/lldb/source/Host/common/FileCache.cpp
@@ -29,12 +29,13 @@ lldb::user_id_t FileCache::OpenFile(const FileSpec &file_spec, uint32_t flags,
     error.SetErrorString("empty path");
     return UINT64_MAX;
   }
-  FileSP file_sp(new File());
-  error = FileSystem::Instance().Open(*file_sp, file_spec, flags, mode);
-  if (!file_sp->IsValid())
+  auto file = FileSystem::Instance().Open(file_spec, flags, mode);
+  if (!file) {
+    error = file.takeError();
     return UINT64_MAX;
-  lldb::user_id_t fd = file_sp->GetDescriptor();
-  m_cache[fd] = file_sp;
+  }
+  lldb::user_id_t fd = file.get()->GetDescriptor();
+  m_cache[fd] = std::move(file.get());
   return fd;
 }
 
@@ -48,12 +49,12 @@ bool FileCache::CloseFile(lldb::user_id_t fd, Status &error) {
     error.SetErrorStringWithFormat("invalid host file descriptor %" PRIu64, fd);
     return false;
   }
-  FileSP file_sp = pos->second;
-  if (!file_sp) {
+  FileUP &file_up = pos->second;
+  if (!file_up) {
     error.SetErrorString("invalid host backing file");
     return false;
   }
-  error = file_sp->Close();
+  error = file_up->Close();
   m_cache.erase(pos);
   return error.Success();
 }
@@ -70,16 +71,16 @@ uint64_t FileCache::WriteFile(lldb::user_id_t fd, uint64_t offset,
     error.SetErrorStringWithFormat("invalid host file descriptor %" PRIu64, fd);
     return false;
   }
-  FileSP file_sp = pos->second;
-  if (!file_sp) {
+  FileUP &file_up = pos->second;
+  if (!file_up) {
     error.SetErrorString("invalid host backing file");
     return UINT64_MAX;
   }
-  if (static_cast<uint64_t>(file_sp->SeekFromStart(offset, &error)) != offset ||
+  if (static_cast<uint64_t>(file_up->SeekFromStart(offset, &error)) != offset ||
       error.Fail())
     return UINT64_MAX;
   size_t bytes_written = src_len;
-  error = file_sp->Write(src, bytes_written);
+  error = file_up->Write(src, bytes_written);
   if (error.Fail())
     return UINT64_MAX;
   return bytes_written;
@@ -96,16 +97,16 @@ uint64_t FileCache::ReadFile(lldb::user_id_t fd, uint64_t offset, void *dst,
     error.SetErrorStringWithFormat("invalid host file descriptor %" PRIu64, fd);
     return false;
   }
-  FileSP file_sp = pos->second;
-  if (!file_sp) {
+  FileUP &file_up = pos->second;
+  if (!file_up) {
     error.SetErrorString("invalid host backing file");
     return UINT64_MAX;
   }
-  if (static_cast<uint64_t>(file_sp->SeekFromStart(offset, &error)) != offset ||
+  if (static_cast<uint64_t>(file_up->SeekFromStart(offset, &error)) != offset ||
       error.Fail())
     return UINT64_MAX;
   size_t bytes_read = dst_len;
-  error = file_sp->Read(dst, bytes_read);
+  error = file_up->Read(dst, bytes_read);
   if (error.Fail())
     return UINT64_MAX;
   return bytes_read;
diff --git a/lldb/source/Host/common/FileSystem.cpp b/lldb/source/Host/common/FileSystem.cpp
index 23b3ebb0b4f..b3442770f01 100644
--- a/lldb/source/Host/common/FileSystem.cpp
+++ b/lldb/source/Host/common/FileSystem.cpp
@@ -415,33 +415,29 @@ static mode_t GetOpenMode(uint32_t permissions) {
   return mode;
 }
 
-Status FileSystem::Open(File &File, const FileSpec &file_spec, uint32_t options,
-                        uint32_t permissions, bool should_close_fd) {
+Expected<FileUP> FileSystem::Open(const FileSpec &file_spec, uint32_t options,
+                                  uint32_t permissions, bool should_close_fd) {
   if (m_collector)
     m_collector->addFile(file_spec.GetPath());
 
-  if (File.IsValid())
-    File.Close();
-
   const int open_flags = GetOpenFlags(options);
   const mode_t open_mode =
       (open_flags & O_CREAT) ? GetOpenMode(permissions) : 0;
 
   auto path = GetExternalPath(file_spec);
   if (!path)
-    return Status(path.getError());
+    return errorCodeToError(path.getError());
 
   int descriptor = llvm::sys::RetryAfterSignal(
       -1, OpenWithFS, *this, path->c_str(), open_flags, open_mode);
 
-  Status error;
-  if (!File::DescriptorIsValid(descriptor)) {
-    File.SetDescriptor(-1, options, false);
-    error.SetErrorToErrno();
-  } else {
-    File.SetDescriptor(descriptor, options, should_close_fd);
-  }
-  return error;
+  if (!File::DescriptorIsValid(descriptor))
+    return llvm::errorCodeToError(
+        std::error_code(errno, std::system_category()));
+
+  auto file = std::make_unique<File>(descriptor, options, should_close_fd);
+  assert(file->IsValid());
+  return std::move(file);
 }
 
 ErrorOr<std::string> FileSystem::GetExternalPath(const llvm::Twine &path) {
diff --git a/lldb/source/Host/windows/Host.cpp b/lldb/source/Host/windows/Host.cpp
index 2ed28066a73..19be271e831 100644
--- a/lldb/source/Host/windows/Host.cpp
+++ b/lldb/source/Host/windows/Host.cpp
@@ -34,9 +34,11 @@ namespace {
 bool GetTripleForProcess(const FileSpec &executable, llvm::Triple &triple) {
   // Open the PE File as a binary file, and parse just enough information to
   // determine the machine type.
-  File imageBinary;
-  FileSystem::Instance().Open(imageBinary, executable, File::eOpenOptionRead,
-                              lldb::eFilePermissionsUserRead);
+  auto imageBinaryP = FileSystem::Instance().Open(
+      executable, File::eOpenOptionRead, lldb::eFilePermissionsUserRead);
+  if (!imageBinaryP)
+    return llvm::errorToBool(imageBinaryP.takeError());
+  File &imageBinary = *imageBinaryP.get();
   imageBinary.SeekFromStart(0x3c);
   int32_t peOffset = 0;
   uint32_t peHead = 0;