Use the non-throwing std::filesystem overloads in os::Utils

vrs/AsyncDiskFileChunk.cpp

@@ -18,6 +18,7 @@
 
 #if VRS_ASYNC_DISKFILE_SUPPORTED()
 
+#include <logging/Checks.h>
 #include <logging/Log.h>
 
 #include <algorithm>
@@ -357,9 +358,11 @@ int AsyncFileDescriptor::close() {
 #endif
 
 AlignedBuffer::AlignedBuffer(size_t size, size_t memalign, size_t lenalign) : capacity_(size) {
-  if (lenalign && 0 != (capacity_ % lenalign)) {
-    throw std::runtime_error("Capacity is not a multiple of lenalign");
-  }
+  XR_CHECK(
+      lenalign == 0 || 0 == (capacity_ % lenalign),
+      "Capacity {} is not a multiple of lenalign {}",
+      capacity_,
+      lenalign);
 #if IS_WINDOWS_PLATFORM()
   aligned_buffer_ = _aligned_malloc(capacity_, memalign);
 #else
@@ -368,10 +371,16 @@ AlignedBuffer::AlignedBuffer(size_t size, size_t memalign, size_t lenalign) : ca
   }
 #endif
   if (aligned_buffer_ == nullptr) {
-    throw std::runtime_error("Failed to allocate aligned buffer");
+    XR_LOGCE(VRS_DISKFILECHUNK, "Failed to allocate a {} byte aligned buffer", capacity_);
+    capacity_ = 0;
   }
 }
 
+std::unique_ptr<AlignedBuffer> AlignedBuffer::make(size_t size, size_t memalign, size_t lenalign) {
+  std::unique_ptr<AlignedBuffer> buffer(new AlignedBuffer(size, memalign, lenalign));
+  return buffer->isValid() ? std::move(buffer) : nullptr;
+}
+
 AlignedBuffer::~AlignedBuffer() {
   free();
 }
@@ -394,21 +403,23 @@ void AlignedBuffer::clear() {
   size_ = 0;
 }
 
-ssize_t AlignedBuffer::add(const void* buffer, size_t size) {
-  assert(size);
-
-  size_t capacity = this->capacity();
-  if (capacity == 0) {
-    return -1;
+bool AlignedBuffer::add(const void* buffer, size_t size, size_t& outCopiedSize) {
+  outCopiedSize = 0;
+  if (!isValid()) {
+    return false;
   }
-  if (size_ >= capacity) {
-    throw std::runtime_error("buffer is already at capacity");
+  size_t tocopy = std::min<size_t>(size, capacity_ - size_);
+  if (tocopy != 0) {
+    memcpy(bdata() + size_, buffer, tocopy);
+    size_ += tocopy;
+    outCopiedSize = tocopy;
   }
-  size_t tocopy = std::min<size_t>(size, capacity - size_);
-  memcpy(bdata() + size_, buffer, tocopy);
-  size_ += tocopy;
+  return true;
+}
 
-  return tocopy;
+std::unique_ptr<AsyncBuffer> AsyncBuffer::make(size_t size, size_t memalign, size_t lenalign) {
+  std::unique_ptr<AsyncBuffer> buffer(new AsyncBuffer(size, memalign, lenalign));
+  return buffer->isValid() ? std::move(buffer) : nullptr;
 }
 
 void AsyncBuffer::complete_write(ssize_t io_return, int io_errno) {
@@ -514,29 +525,29 @@ void AsyncBuffer::SigEvNotifyFunction(union sigval val) {
   ssize_t io_return = 0;
   int io_errno = 0;
 
+  // Runs on a libc SIGEV_THREAD thread.
   io_errno = aio_error(&self->aiocb_);
   if (io_errno == 0) {
     io_return = aio_return(&self->aiocb_);
-    if (io_return < 0) {
-      throw std::runtime_error(
-          "aio_return returned a negative number despite aio_error indicating success");
-    }
+    XR_CHECK_GE(
+        io_return, 0, "aio_return returned {} despite aio_error indicating success", io_return);
   } else if (io_errno == EINPROGRESS) {
-    throw std::runtime_error("aio_error()==EINPROGRESS on a completed aio_write");
+    XR_CHECK(false, "aio_error()==EINPROGRESS on a completed aio_write");
   } else if (io_errno == ECANCELED) {
     // If canceled, aio_return will give -1
     io_return = aio_return(&self->aiocb_);
-    if (io_return >= 0) {
-      throw std::runtime_error(
-          "aio_error() signaled cancellation, but aio_return indicated success");
-    }
+    XR_CHECK_LT(
+        io_return, 0, "aio_error() signaled cancellation, but aio_return returned {}", io_return);
   } else if (io_errno > 0) {
     io_return = aio_return(&self->aiocb_);
-    if (io_return >= 0) {
-      throw std::runtime_error("aio_error() signaled an error, but aio_return indicated success");
-    }
+    XR_CHECK_LT(
+        io_return,
+        0,
+        "aio_error() signaled error {}, but aio_return returned {}",
+        io_errno,
+        io_return);
   } else {
-    throw std::runtime_error("aio_error() returned an unexpected negative number");
+    XR_CHECK(false, "aio_error() returned an unexpected negative number: {}", io_errno);
   }
 
   self->complete_write(io_return, io_errno);
@@ -579,10 +590,9 @@ AsyncDiskFileChunk::AsyncDiskFileChunk(AsyncDiskFileChunk&& other) noexcept {
 }
 
 AsyncDiskFileChunk::~AsyncDiskFileChunk() {
-  try {
-    close();
-  } catch (std::exception& e) {
-    XR_LOGCE(VRS_DISKFILECHUNK, "Exception on close() during destruction: {}", e.what());
+  int error = close();
+  if (error != 0) {
+    XR_LOGCE(VRS_DISKFILECHUNK, "close() failed during destruction: {}", errorCodeToMessage(error));
   }
 }
 
@@ -725,8 +735,8 @@ int AsyncDiskFileChunk::write(const void* buffer, size_t count, size_t& outWritt
 
   while (count != 0) {
     // This data is aligned to lenalign, so cache it in the current_buffer_
-    ssize_t additionalBuffered = current_buffer_->add(bbuffer, count);
-    if (additionalBuffered <= 0) {
+    size_t additionalBuffered = 0;
+    if (!current_buffer_->add(bbuffer, count, additionalBuffered)) {
       return DISKFILE_PARTIAL_WRITE_ERROR;
     }
     bbuffer += additionalBuffered;
@@ -1044,7 +1054,7 @@ int AsyncDiskFileChunk::alloc_write_buffers() {
   buffers_free_.reserve(num_buffers_);
   buffers_.reserve(num_buffers_);
   while (buffers_.size() < num_buffers_) {
-    auto buffer = std::make_unique<AsyncBuffer>(buffer_size_, mem_align_, offset_align_);
+    auto buffer = AsyncBuffer::make(buffer_size_, mem_align_, offset_align_);
     if (!buffer) {
       return ENOMEM;
     }

vrs/AsyncDiskFileChunk.h

@@ -130,8 +130,19 @@ class VRS_API AlignedBuffer {
   size_t capacity_ = 0;
   size_t size_ = 0;
 
- public:
+ protected:
+  /// `size` must be a multiple of `lenalign`, unless `lenalign` is 0.
   AlignedBuffer(size_t size, size_t memalign, size_t lenalign);
+
+  [[nodiscard]] inline bool isValid() const {
+    return aligned_buffer_ != nullptr;
+  }
+
+ public:
+  /// @return A buffer of `size` bytes aligned to `memalign`, or nullptr if it couldn't be
+  /// allocated.
+  static std::unique_ptr<AlignedBuffer> make(size_t size, size_t memalign, size_t lenalign);
+
   virtual ~AlignedBuffer();
 
   [[nodiscard]] inline size_t size() const {
@@ -155,7 +166,11 @@ class VRS_API AlignedBuffer {
   [[nodiscard]] inline char* bdata() const {
     return reinterpret_cast<char*>(aligned_buffer_);
   }
-  [[nodiscard]] ssize_t add(const void* buffer, size_t size);
+  /// Append up to `size` bytes, stopping at the buffer's capacity.
+  /// @param outCopiedSize: Set to the number of bytes copied, which is 0 if the buffer is
+  /// already full, or `size` was 0.
+  /// @return False if the buffer has no storage.
+  [[nodiscard]] bool add(const void* buffer, size_t size, size_t& outCopiedSize);
 };
 
 class AsyncBuffer;
@@ -171,14 +186,20 @@ class VRS_API AsyncBuffer : public AlignedBuffer {
  public:
   using complete_write_callback = std::function<void(ssize_t io_return, int io_errno)>;
 
-  AsyncBuffer(size_t size, size_t memalign, size_t lenalign)
-      : AlignedBuffer(size, memalign, lenalign) {}
+  /// @return A buffer of `size` bytes aligned to `memalign`, or nullptr if it couldn't be
+  /// allocated.
+  static std::unique_ptr<AsyncBuffer> make(size_t size, size_t memalign, size_t lenalign);
+
   ~AsyncBuffer() override = default;
 
   void complete_write(ssize_t io_return, int io_errno);
   [[nodiscard]] int
   start_write(const AsyncHandle& file, int64_t offset, complete_write_callback on_complete);
 
+ protected:
+  AsyncBuffer(size_t size, size_t memalign, size_t lenalign)
+      : AlignedBuffer(size, memalign, lenalign) {}
+
  private:
 #if IS_WINDOWS_PLATFORM()
   AsyncOVERLAPPED ov_;

vrs/os/Utils.cpp

@@ -231,11 +231,15 @@ const string& getTempFolder() {
 /// Returns true if the source was really a link, but you can *always* use outLinkedPath.
 bool getLinkedTarget(const string& sourcePath, string& outLinkedPath) {
   fs::path source(sourcePath);
-  if (fs::is_symlink(source)) {
+  fs_error_code ec;
+  if (fs::is_symlink(source, ec) && !ec) {
     // Note: apply canonical() instead of readlink()
     // so that relative paths in symlinks are resolved properly
-    outLinkedPath = fs::canonical(source).string();
-    return true;
+    fs::path target = fs::canonical(source, ec);
+    if (!ec) {
+      outLinkedPath = target.string();
+      return true;
+    }
   }
   outLinkedPath = sourcePath;
   return false;
@@ -380,10 +384,13 @@ bool isFile(const string& path) {
 
 vector<string> listDir(const string& dir) {
   vector<string> result;
-  if (isDir(dir)) {
-    for (const auto& entry : fs::directory_iterator(dir)) {
-      result.push_back(entry.path().string());
-    }
+  fs_error_code ec;
+  for (fs::directory_iterator it(dir, ec), end; !ec && it != end; it.increment(ec)) {
+    result.push_back(it->path().string());
+  }
+  if (ec) {
+    // A partial listing is indistinguishable from a complete one, so report nothing instead.
+    result.clear();
   }
   return result;
 }

vrs/os/test/UtilsTest.cpp

@@ -17,6 +17,8 @@
 #include "vrs/os/Utils.h"
 
 #include <algorithm>
+#include <cerrno>
+#include <filesystem>
 #include <fstream>
 #include <vector>
 
@@ -27,6 +29,10 @@
 #include <vrs/os/Platform.h>
 #include <vrs/os/System.h>
 
+#if !IS_WINDOWS_PLATFORM() && !IS_ANDROID_PLATFORM()
+#include <unistd.h>
+#endif
+
 // This test verifies that the following definitions are available for... tests!!
 // Note: the file is generated by cmake, so do not run this when building from a solution file...
 
@@ -38,6 +44,12 @@ struct FileTest : testing::Test {
 
   string testDataDir = coretech::getTestDataDir();
 
+  static void createEmptyFile(const string& path) {
+    FILE* file = os::fileOpen(path, "w");
+    ASSERT_NE(file, nullptr) << "could not create " << path;
+    ASSERT_EQ(os::fileClose(file), 0);
+  }
+
   static void testFileName(const string& filename) {
     string path = os::pathJoin(os::makeUniqueFolder(), os::sanitizeFileName(filename));
     int status = os::makeDir(path);
@@ -253,6 +265,66 @@ TEST_F(FileTest, testListDir) {
   EXPECT_EQ(files, expectedFiles);
 }
 
+TEST_F(FileTest, testListDirOnMissingPathIsEmpty) {
+  const string missing = os::pathJoin(os::makeUniqueFolder(), "no_such_folder");
+  ASSERT_FALSE(os::pathExists(missing));
+  EXPECT_TRUE(os::listDir(missing).empty());
+}
+
+TEST_F(FileTest, testListDirOnAFileIsEmpty) {
+  const string path = os::pathJoin(os::makeUniqueFolder(), "not_a_folder.txt");
+  ASSERT_NO_FATAL_FAILURE(createEmptyFile(path));
+  EXPECT_TRUE(os::listDir(path).empty());
+}
+
+TEST_F(FileTest, testGetLinkedTargetOfARegularFile) {
+  const string path = os::pathJoin(os::makeUniqueFolder(), "regular.txt");
+  ASSERT_NO_FATAL_FAILURE(createEmptyFile(path));
+  string linkedPath;
+  EXPECT_FALSE(os::getLinkedTarget(path, linkedPath));
+  EXPECT_EQ(linkedPath, path);
+}
+
+TEST_F(FileTest, testGetLinkedTargetOfAMissingPath) {
+  const string missing = os::pathJoin(os::makeUniqueFolder(), "no_such_file.txt");
+  ASSERT_FALSE(os::pathExists(missing));
+  string linkedPath;
+  EXPECT_FALSE(os::getLinkedTarget(missing, linkedPath));
+  EXPECT_EQ(linkedPath, missing);
+}
+
+// Windows has no symlinks, and Android's temp folder may not permit creating them.
+#if !IS_WINDOWS_PLATFORM() && !IS_ANDROID_PLATFORM()
+TEST_F(FileTest, testGetLinkedTargetResolvesASymlink) {
+  // On macOS the temp folder lives under /var, itself a symlink to /private/var.
+  std::error_code ec;
+  const string folder = std::filesystem::canonical(os::makeUniqueFolder(), ec).string();
+  ASSERT_FALSE(ec) << ec.message();
+  const string target = os::pathJoin(folder, "target.txt");
+  const string link = os::pathJoin(folder, "link.txt");
+  ASSERT_NO_FATAL_FAILURE(createEmptyFile(target));
+  const int symlinkResult = ::symlink(target.c_str(), link.c_str());
+  const int symlinkErrno = errno;
+  ASSERT_EQ(symlinkResult, 0) << "symlink failed, errno " << symlinkErrno;
+  string linkedPath;
+  EXPECT_TRUE(os::getLinkedTarget(link, linkedPath));
+  EXPECT_EQ(linkedPath, target);
+}
+
+TEST_F(FileTest, testGetLinkedTargetOfABrokenSymlink) {
+  const string folder = os::makeUniqueFolder();
+  const string missingTarget = os::pathJoin(folder, "gone.txt");
+  const string link = os::pathJoin(folder, "broken_link.txt");
+  const int symlinkResult = ::symlink(missingTarget.c_str(), link.c_str());
+  const int symlinkErrno = errno;
+  ASSERT_EQ(symlinkResult, 0) << "symlink failed, errno " << symlinkErrno;
+  ASSERT_FALSE(os::pathExists(missingTarget));
+  string linkedPath;
+  EXPECT_FALSE(os::getLinkedTarget(link, linkedPath));
+  EXPECT_EQ(linkedPath, link);
+}
+#endif // !IS_WINDOWS_PLATFORM() && !IS_ANDROID_PLATFORM()
+
 TEST(System, getTerminalWidthTest) {
   EXPECT_EQ(os::getTerminalWidth(120), 120); // set the value
   EXPECT_EQ(os::getTerminalWidth(), 120); // get the value back