Add new PathError to mimic InodeError's behavior with a raw path

Summary: Upcoming work needs to return errors that look like `InodeError` today, but are rooted in a path, not an inode. Move most of the functionality of `InodeError` into a new `PathErrorBase` class, and then rework `InodeError` to inherit from that. Also add the new `PathError` for use later.

Reviewed By: xavierd

Differential Revision: D35517826

fbshipit-source-id: 679505e4fd9b1d1548f7bb2d7a2f6dab0b12c6ab

eden/fs/inodes/InodeError.cpp

@@ -10,11 +10,10 @@
 #include <folly/String.h>
 #include "eden/fs/inodes/TreeInode.h"
 
-namespace facebook {
-namespace eden {
+namespace facebook::eden {
 
 InodeError::InodeError(int errnum, TreeInodePtr inode, PathComponentPiece child)
-    : std::system_error(errnum, std::generic_category()),
+    : PathErrorBase(errnum),
       inode_(std::move(inode)),
       child_(PathComponent{child}) {}
 
@@ -23,26 +22,11 @@ InodeError::InodeError(
     TreeInodePtr inode,
     PathComponentPiece child,
     std::string&& message)
-    : std::system_error(errnum, std::generic_category()),
+    : PathErrorBase(errnum, std::move(message)),
       inode_(std::move(inode)),
-      child_(PathComponent{child}),
-      message_(std::move(message)) {}
+      child_(PathComponent{child}) {}
 
-const char* InodeError::what() const noexcept {
-  try {
-    auto msg = fullMessage_.wlock();
-    if (msg->empty()) {
-      *msg = computeMessage();
-    }
-
-    return msg->c_str();
-  } catch (...) {
-    // Fallback value if anything goes wrong building the real message
-    return "<InodeError>";
-  }
-}
-
-std::string InodeError::computeMessage() const {
+std::string InodeError::computePath() const noexcept {
   std::string path;
   if (inode_) {
     if (child_.has_value()) {
@@ -57,11 +41,6 @@ std::string InodeError::computeMessage() const {
       path = inode_->getLogPath();
     }
   }
-
-  if (message_.empty()) {
-    return path + ": " + folly::errnoStr(errnum());
-  }
-  return path + ": " + message_ + ": " + folly::errnoStr(errnum());
+  return path;
 }
-} // namespace eden
-} // namespace facebook
+} // namespace facebook::eden

eden/fs/inodes/InodeError.h

@@ -7,35 +7,27 @@
 
 #pragma once
 
-#include <folly/Format.h>
-#include <folly/Synchronized.h>
-#include <memory>
-#include <optional>
-#include <system_error>
 #include "eden/fs/inodes/InodePtr.h"
+#include "eden/fs/inodes/PathError.h"
 #include "eden/fs/utils/PathFuncs.h"
 
-namespace facebook {
-namespace eden {
+namespace facebook::eden {
 
 /**
- * A subclass of std::system_error referring to a specific inode.
+ * A subclass of PathErrorBase referring to a specific inode.
  *
  * The main advantage of this class is that it can include the Inode path in
  * the error message.  However, it avoids computing the path until the error
  * message is actually needed.  If the error is caught and handled without
  * looking at the error message, then the path never needs to be computed.
  */
-class InodeError : public std::system_error {
+class InodeError : public PathErrorBase {
  public:
   InodeError(int errnum, InodePtr inode)
-      : std::system_error(errnum, std::generic_category()),
-        inode_(std::move(inode)) {}
+      : PathErrorBase(errnum), inode_(std::move(inode)) {}
   InodeError(int errnum, TreeInodePtr inode, PathComponentPiece child);
   InodeError(int errnum, InodePtr inode, std::string message)
-      : std::system_error(errnum, std::generic_category()),
-        inode_(std::move(inode)),
-        message_(std::move(message)) {}
+      : PathErrorBase(errnum, std::move(message)), inode_(std::move(inode)) {}
   InodeError(
       int errnum,
       TreeInodePtr inode,
@@ -62,26 +54,19 @@ class InodeError : public std::system_error {
             errnum,
             inode,
             child,
-            message_(folly::sformat(format, std::forward<Args>(args)...))) {}
-
-  const char* what() const noexcept override;
-
-  int errnum() const {
-    return code().value();
-  }
+            folly::sformat(format, std::forward<Args>(args)...)) {}
+  ~InodeError() override = default;
 
   InodeError(InodeError const&) = default;
   InodeError& operator=(InodeError const&) = default;
   InodeError(InodeError&&) = default;
   InodeError& operator=(InodeError&&) = default;
 
- private:
-  std::string computeMessage() const;
+ protected:
+  std::string computePath() const noexcept override;
 
+ private:
   InodePtr inode_;
   std::optional<PathComponent> child_;
-  std::string message_;
-  mutable folly::Synchronized<std::string> fullMessage_;
 };
-} // namespace eden
-} // namespace facebook
+} // namespace facebook::eden

eden/fs/inodes/InodePtr.cpp

@@ -14,8 +14,8 @@
 #include "eden/fs/inodes/InodeError.h"
 #include "eden/fs/inodes/TreeInode.h"
 
-namespace facebook {
-namespace eden {
+namespace facebook::eden {
+
 template <typename SubclassRawPtrType>
 SubclassRawPtrType InodePtr::asSubclass() const {
   if (this->value_ == nullptr) {
@@ -136,10 +136,13 @@ TreeInodePtr InodePtr::asTreePtrOrNull() && {
   return extractSubclassPtrOrNull<TreeInodePtr>();
 }
 
+InodePtr InodePtr::takeOwnership(std::unique_ptr<InodeBase> value) noexcept {
+  return InodePtr::newPtrLocked(value.release());
+}
+
 // Explicitly instantiate InodePtrImpl for all inode class types
 template class InodePtrImpl<FileInode>;
 template class InodePtrImpl<TreeInode>;
 template FileInodePtr InodePtr::asSubclassPtrOrNull<FileInodePtr>() const&;
 template TreeInodePtr InodePtr::asSubclassPtrOrNull<TreeInodePtr>() const&;
-} // namespace eden
-} // namespace facebook
+} // namespace facebook::eden

eden/fs/inodes/InodePtr.h

@@ -14,8 +14,7 @@
 #include <utility>
 #include "folly/logging/xlog.h"
 
-namespace facebook {
-namespace eden {
+namespace facebook::eden {
 
 /**
  * A custom smart pointer class for pointing to Inode objects.
@@ -246,9 +245,7 @@ class InodePtr : public InodePtrImpl<InodeBase> {
   static InodePtr newPtrLocked(InodeBase* value) noexcept {
     return InodePtr{value, InodePtrImpl<InodeBase>::LOCKED_INCREMENT};
   }
-  static InodePtr takeOwnership(std::unique_ptr<InodeBase> value) noexcept {
-    return InodePtr::newPtrLocked(value.release());
-  }
+  static InodePtr takeOwnership(std::unique_ptr<InodeBase> value) noexcept;
   static InodePtr newPtrFromExisting(InodeBase* value) noexcept {
     return InodePtr{value, InodePtrImpl<InodeBase>::NORMAL_INCREMENT};
   }
@@ -377,5 +374,4 @@ template <typename InodeTypeParam>
 bool operator!=(std::nullptr_t, const InodePtrImpl<InodeTypeParam>& ptr) {
   return bool(ptr);
 }
-} // namespace eden
-} // namespace facebook
+} // namespace facebook::eden

eden/fs/inodes/PathError.cpp

@@ -0,0 +1,35 @@
+/*
+ * Copyright (c) Meta Platforms, Inc. and affiliates.
+ *
+ * This software may be used and distributed according to the terms of the
+ * GNU General Public License version 2.
+ */
+
+#include "eden/fs/inodes/PathError.h"
+
+#include <folly/String.h>
+
+namespace facebook::eden {
+
+const char* PathErrorBase::what() const noexcept {
+  try {
+    auto msg = fullMessage_.wlock();
+    if (msg->empty()) {
+      *msg = computeMessage();
+    }
+
+    return msg->c_str();
+  } catch (...) {
+    // Fallback value if anything goes wrong building the real message
+    return "<PathErrorBase>";
+  }
+}
+
+std::string PathErrorBase::computeMessage() const {
+  std::string path = computePath().c_str();
+  if (message_.empty()) {
+    return path + ": " + folly::errnoStr(code().value());
+  }
+  return path + ": " + message_ + ": " + folly::errnoStr(code().value());
+}
+} // namespace facebook::eden

eden/fs/inodes/PathError.h

@@ -0,0 +1,94 @@
+/*
+ * Copyright (c) Meta Platforms, Inc. and affiliates.
+ *
+ * This software may be used and distributed according to the terms of the
+ * GNU General Public License version 2.
+ */
+
+#pragma once
+
+#include <folly/Format.h>
+#include <folly/Synchronized.h>
+#include <memory>
+#include <optional>
+#include <system_error>
+
+#include "eden/fs/utils/PathFuncs.h"
+
+namespace facebook::eden {
+
+/**
+ * A subclass of std::system_error referring to a specific path.
+ *
+ * The main advantage of this class is that it can include a path in
+ * the error message.  However, it avoids computing the subclass's path until
+ * the error message is actually needed.  If the error is caught and handled
+ * without looking at the error message, then the path never needs to be
+ * computed.
+ */
+class PathErrorBase : public std::system_error {
+ public:
+  explicit PathErrorBase(int errnum)
+      : std::system_error(errnum, std::generic_category()) {}
+  PathErrorBase(int errnum, std::string message)
+      : std::system_error(errnum, std::generic_category()),
+        message_(std::move(message)) {}
+  ~PathErrorBase() override = default;
+
+  const char* what() const noexcept override;
+
+  PathErrorBase(PathErrorBase const&) = default;
+  PathErrorBase& operator=(PathErrorBase const&) = default;
+  PathErrorBase(PathErrorBase&&) = default;
+  PathErrorBase& operator=(PathErrorBase&&) = default;
+
+ protected:
+  virtual std::string computePath() const noexcept = 0;
+
+ private:
+  std::string computeMessage() const;
+
+  std::string message_;
+  mutable folly::Synchronized<std::string> fullMessage_;
+};
+
+/**
+ * A subclass of PathErrorBase referring to a specific path by string.
+ *
+ * Users should prefer InodeError to avoid copying and storing a string
+ * unecesarily, but an inode isn't always available where PathErrorBase errors
+ * are needed.
+ */
+class PathError : public PathErrorBase {
+ public:
+  explicit PathError(int errnum, RelativePathPiece path, std::string message)
+      : PathErrorBase(errnum, std::move(message)), path_(path.copy()) {}
+  explicit PathError(int errnum, RelativePathPiece path)
+      : PathErrorBase(errnum), path_(path.copy()) {}
+  ~PathError() override = default;
+
+  template <typename... Args>
+  PathError(
+      int errnum,
+      const folly::StringPiece path,
+      folly::StringPiece format,
+      Args&&... args)
+      : PathError(
+            errnum,
+            path,
+            folly::sformat(format, std::forward<Args>(args)...)) {}
+
+  PathError(PathError const&) = default;
+  PathError& operator=(PathError const&) = default;
+  PathError(PathError&&) = default;
+  PathError& operator=(PathError&&) = default;
+
+ protected:
+  std::string computePath() const noexcept override {
+    return path_.stringPiece().str();
+  }
+
+ private:
+  RelativePath path_;
+};
+} // namespace facebook::eden