refactor: drop Expected readlink API for now

include/dwarfs/filesystem_v2.h

@@ -30,6 +30,7 @@
 #include <optional>
 #include <span>
 #include <string>
+#include <system_error>
 #include <utility>
 
 #include <folly/Expected.h>
@@ -37,6 +38,7 @@
 #include <nlohmann/json.hpp>
 
 #include <dwarfs/block_range.h>
+#include <dwarfs/file_stat.h>
 #include <dwarfs/metadata_types.h>
 #include <dwarfs/options.h>
 #include <dwarfs/types.h>
@@ -44,7 +46,6 @@
 namespace dwarfs {
 
 struct iovec_read_buf;
-struct file_stat;
 struct vfs_stat;
 
 class category_resolver;
@@ -139,9 +140,17 @@ class filesystem_v2 {
     return impl_->readlink(entry, buf, mode);
   }
 
-  folly::Expected<std::string, int>
+  std::string
-  readlink(inode_view entry,
+  readlink(inode_view entry, readlink_mode mode, std::error_code& ec) const {
-           readlink_mode mode = readlink_mode::preferred) const {
+    return impl_->readlink(entry, mode, ec);
+  }
+
+  std::string readlink(inode_view entry, std::error_code& ec) const {
+    return impl_->readlink(entry, readlink_mode::preferred, ec);
+  }
+
+  std::string readlink(inode_view entry,
+                       readlink_mode mode = readlink_mode::preferred) const {
     return impl_->readlink(entry, mode);
   }
 
@@ -229,7 +238,9 @@ class filesystem_v2 {
     virtual size_t dirsize(directory_view dir) const = 0;
     virtual int
     readlink(inode_view entry, std::string* buf, readlink_mode mode) const = 0;
-    virtual folly::Expected<std::string, int>
+    virtual std::string readlink(inode_view entry, readlink_mode mode,
+                                 std::error_code& ec) const = 0;
+    virtual std::string
     readlink(inode_view entry, readlink_mode mode) const = 0;
     virtual int statvfs(vfs_stat* stbuf) const = 0;
     virtual int open(inode_view entry) const = 0;

include/dwarfs/metadata_v2.h

@@ -32,8 +32,6 @@
 #include <utility>
 #include <vector>
 
-#include <folly/Expected.h>
-
 #include <nlohmann/json.hpp>
 
 #include <dwarfs/metadata_types.h>
@@ -128,11 +126,6 @@ class metadata_v2 {
     return impl_->readlink(iv, buf, mode);
   }
 
-  folly::Expected<std::string, int>
-  readlink(inode_view iv, readlink_mode mode) const {
-    return impl_->readlink(iv, mode);
-  }
-
   int statvfs(vfs_stat* stbuf) const { return impl_->statvfs(stbuf); }
 
   std::optional<chunk_range> get_chunks(int inode) const {
@@ -208,9 +201,6 @@ class metadata_v2 {
     virtual int
     readlink(inode_view iv, std::string* buf, readlink_mode mode) const = 0;
 
-    virtual folly::Expected<std::string, int>
-    readlink(inode_view iv, readlink_mode mode) const = 0;
-
     virtual int statvfs(vfs_stat* stbuf) const = 0;
 
     virtual std::optional<chunk_range> get_chunks(int inode) const = 0;

src/dwarfs/filesystem_extractor.cpp

@@ -364,9 +364,10 @@ bool filesystem_extractor_<LoggerPolicy>::extract(
     ::archive_entry_copy_stat(ae, &st);
 
     if (inode.is_symlink()) {
-      std::string link;
+      std::error_code ec;
-      if (fs.readlink(inode, &link) != 0) {
+      auto link = fs.readlink(inode, ec);
-        LOG_ERROR << "readlink() failed";
+      if (ec) {
+        LOG_ERROR << "readlink() failed: " << ec.message();
       }
       if (opts.progress) {
         bytes_written += link.size();

src/dwarfs/filesystem_v2.cpp

@@ -75,6 +75,28 @@ void check_section_logger(logger& lgr, fs_section const& section) {
   }
 }
 
+template <typename Fn>
+auto call_ec_throw(Fn&& fn) {
+  std::error_code ec;
+  auto result = std::forward<Fn>(fn)(ec);
+  if (ec) {
+    throw std::system_error(ec);
+  }
+  return result;
+}
+
+template <typename R, typename Fn>
+R call_int_error(Fn&& fn, std::error_code& ec) {
+  R res;
+  auto err = std::forward<Fn>(fn)(res);
+  if (err < 0) {
+    ec = std::error_code{-err, std::system_category()};
+  } else {
+    ec.clear();
+  }
+  return res;
+}
+
 class filesystem_parser {
  private:
   static uint64_t constexpr section_offset_mask{(UINT64_C(1) << 48) - 1};
@@ -410,8 +432,9 @@ class filesystem_ final : public filesystem_v2::impl {
   size_t dirsize(directory_view dir) const override;
   int readlink(inode_view entry, std::string* buf,
                readlink_mode mode) const override;
-  folly::Expected<std::string, int>
+  std::string readlink(inode_view entry, readlink_mode mode,
-  readlink(inode_view entry, readlink_mode mode) const override;
+                       std::error_code& ec) const override;
+  std::string readlink(inode_view entry, readlink_mode mode) const override;
   int statvfs(vfs_stat* stbuf) const override;
   int open(inode_view entry) const override;
   ssize_t read(uint32_t inode, char* buf, size_t size,
@@ -447,6 +470,8 @@ class filesystem_ final : public filesystem_v2::impl {
  private:
   filesystem_info const& get_info() const;
   void check_section(fs_section const& section) const;
+  std::string
+  readlink_ec(inode_view entry, readlink_mode mode, std::error_code& ec) const;
 
   LOG_PROXY_DECL(LoggerPolicy);
   os_access const& os_;
@@ -469,7 +494,8 @@ class filesystem_ final : public filesystem_v2::impl {
   PERFMON_CLS_TIMER_DECL(readdir)
   PERFMON_CLS_TIMER_DECL(dirsize)
   PERFMON_CLS_TIMER_DECL(readlink)
-  PERFMON_CLS_TIMER_DECL(readlink_expected)
+  PERFMON_CLS_TIMER_DECL(readlink_ec)
+  PERFMON_CLS_TIMER_DECL(readlink_throw)
   PERFMON_CLS_TIMER_DECL(statvfs)
   PERFMON_CLS_TIMER_DECL(open)
   PERFMON_CLS_TIMER_DECL(read)
@@ -547,7 +573,8 @@ filesystem_<LoggerPolicy>::filesystem_(
     PERFMON_CLS_TIMER_INIT(readdir)
     PERFMON_CLS_TIMER_INIT(dirsize)
     PERFMON_CLS_TIMER_INIT(readlink)
-    PERFMON_CLS_TIMER_INIT(readlink_expected)
+    PERFMON_CLS_TIMER_INIT(readlink_ec)
+    PERFMON_CLS_TIMER_INIT(readlink_throw)
     PERFMON_CLS_TIMER_INIT(statvfs)
     PERFMON_CLS_TIMER_INIT(open)
     PERFMON_CLS_TIMER_INIT(read)
@@ -1049,11 +1076,27 @@ int filesystem_<LoggerPolicy>::readlink(inode_view entry, std::string* buf,
 }
 
 template <typename LoggerPolicy>
-folly::Expected<std::string, int>
+std::string
-filesystem_<LoggerPolicy>::readlink(inode_view entry,
+filesystem_<LoggerPolicy>::readlink_ec(inode_view entry, readlink_mode mode,
-                                    readlink_mode mode) const {
+                                       std::error_code& ec) const {
-  PERFMON_CLS_SCOPED_SECTION(readlink_expected)
+  return call_int_error<std::string>(
-  return meta_.readlink(entry, mode);
+      [&](auto& buf) { return meta_.readlink(entry, &buf, mode); }, ec);
+}
+
+template <typename LoggerPolicy>
+std::string
+filesystem_<LoggerPolicy>::readlink(inode_view entry, readlink_mode mode,
+                                    std::error_code& ec) const {
+  PERFMON_CLS_SCOPED_SECTION(readlink_ec)
+  return readlink_ec(entry, mode, ec);
+}
+
+template <typename LoggerPolicy>
+std::string filesystem_<LoggerPolicy>::readlink(inode_view entry,
+                                                readlink_mode mode) const {
+  PERFMON_CLS_SCOPED_SECTION(readlink_throw)
+  return call_ec_throw(
+      [&](std::error_code& ec) { return readlink_ec(entry, mode, ec); });
 }
 
 template <typename LoggerPolicy>

src/dwarfs/metadata_v2.cpp

@@ -499,9 +499,6 @@ class metadata_ final : public metadata_v2::impl {
   int readlink(inode_view iv, std::string* buf,
                readlink_mode mode) const override;
 
-  folly::Expected<std::string, int>
-  readlink(inode_view iv, readlink_mode mode) const override;
-
   int statvfs(vfs_stat* stbuf) const override;
 
   std::optional<chunk_range> get_chunks(int inode) const override;
@@ -1695,16 +1692,6 @@ int metadata_<LoggerPolicy>::readlink(inode_view iv, std::string* buf,
   return -EINVAL;
 }
 
-template <typename LoggerPolicy>
-folly::Expected<std::string, int>
-metadata_<LoggerPolicy>::readlink(inode_view iv, readlink_mode mode) const {
-  if (iv.is_symlink()) {
-    return link_value(iv, mode);
-  }
-
-  return folly::makeUnexpected(-EINVAL);
-}
-
 template <typename LoggerPolicy>
 int metadata_<LoggerPolicy>::statvfs(vfs_stat* stbuf) const {
   ::memset(stbuf, 0, sizeof(*stbuf));

src/dwarfsck_main.cpp

@@ -85,7 +85,7 @@ void do_list_files(filesystem_v2& fs, iolayer const& iol, bool verbose) {
 
     if (verbose) {
       if (iv.is_symlink()) {
-        auto target = fs.readlink(iv).value();
+        auto target = fs.readlink(iv);
         utf8_sanitize(target);
         name += " -> " + target;
       }

test/filesystem_test.cpp

@@ -102,17 +102,30 @@ TEST(filesystem, metadata_symlink_win) {
     EXPECT_EQ(-EINVAL, fs.readlink(*i3, &buffer));
   }
 
-  // also test expected interface
+  // also test error_code interface
   {
-    auto r = fs.readlink(*i1, readlink_mode::unix);
+    std::error_code ec;
-    EXPECT_TRUE(r);
+    auto r = fs.readlink(*i1, readlink_mode::unix, ec);
-    EXPECT_EQ("subdir/test.txt", *r);
+    EXPECT_FALSE(ec);
+    EXPECT_EQ("subdir/test.txt", r);
   }
 
   {
-    auto r = fs.readlink(*i3);
+    std::error_code ec;
-    EXPECT_FALSE(r);
+    auto r = fs.readlink(*i3, ec);
-    EXPECT_EQ(-EINVAL, r.error());
+    EXPECT_TRUE(ec);
+    EXPECT_EQ(EINVAL, ec.value());
+  }
+
+  // also test throwing interface
+  {
+    auto r = fs.readlink(*i1, readlink_mode::unix);
+    EXPECT_EQ("subdir/test.txt", r);
+  }
+
+  {
+    EXPECT_THAT([&] { fs.readlink(*i3); },
+                ::testing::Throws<std::system_error>());
   }
 }
 
@@ -172,17 +185,19 @@ TEST(filesystem, metadata_symlink_unix) {
     EXPECT_EQ(-EINVAL, fs.readlink(*i3, &buffer));
   }
 
-  // also test expected interface
+  // also test error_code interface
   {
-    auto r = fs.readlink(*i1, readlink_mode::unix);
+    std::error_code ec;
-    EXPECT_TRUE(r);
+    auto r = fs.readlink(*i1, readlink_mode::unix, ec);
-    EXPECT_EQ("subdir/test.txt", *r);
+    EXPECT_FALSE(ec);
+    EXPECT_EQ("subdir/test.txt", r);
   }
 
   {
-    auto r = fs.readlink(*i3);
+    std::error_code ec;
-    EXPECT_FALSE(r);
+    auto r = fs.readlink(*i3, ec);
-    EXPECT_EQ(-EINVAL, r.error());
+    EXPECT_TRUE(ec);
+    EXPECT_EQ(EINVAL, ec.value());
   }
 }
 

test/tool_main_test.cpp

@@ -537,7 +537,7 @@ TEST(dwarfsextract_test, perfmon) {
   EXPECT_THAT(errs, ::testing::HasSubstr("[filesystem_v2.readv_future]"));
   EXPECT_THAT(errs, ::testing::HasSubstr("[filesystem_v2.getattr]"));
   EXPECT_THAT(errs, ::testing::HasSubstr("[filesystem_v2.open]"));
-  EXPECT_THAT(errs, ::testing::HasSubstr("[filesystem_v2.readlink]"));
+  EXPECT_THAT(errs, ::testing::HasSubstr("[filesystem_v2.readlink_ec]"));
   EXPECT_THAT(errs, ::testing::HasSubstr("[filesystem_v2.statvfs]"));
   EXPECT_THAT(errs, ::testing::HasSubstr("[inode_reader_v2.readv_future]"));
   static std::regex const perfmon_re{R"(\[filesystem_v2\.getattr\])"