diff --git a/include/dwarfs/filesystem_v2.h b/include/dwarfs/filesystem_v2.h index eb578d58..9afc0ef3 100644 --- a/include/dwarfs/filesystem_v2.h +++ b/include/dwarfs/filesystem_v2.h @@ -30,6 +30,7 @@ #include #include #include +#include #include #include @@ -37,6 +38,7 @@ #include #include +#include #include #include #include @@ -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 - readlink(inode_view entry, - readlink_mode mode = readlink_mode::preferred) const { + std::string + readlink(inode_view entry, readlink_mode mode, std::error_code& ec) 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 + 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; diff --git a/include/dwarfs/metadata_v2.h b/include/dwarfs/metadata_v2.h index a7cb6ddf..bc5ebe0e 100644 --- a/include/dwarfs/metadata_v2.h +++ b/include/dwarfs/metadata_v2.h @@ -32,8 +32,6 @@ #include #include -#include - #include #include @@ -128,11 +126,6 @@ class metadata_v2 { return impl_->readlink(iv, buf, mode); } - folly::Expected - 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 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 - readlink(inode_view iv, readlink_mode mode) const = 0; - virtual int statvfs(vfs_stat* stbuf) const = 0; virtual std::optional get_chunks(int inode) const = 0; diff --git a/src/dwarfs/filesystem_extractor.cpp b/src/dwarfs/filesystem_extractor.cpp index 2124a888..8606b26f 100644 --- a/src/dwarfs/filesystem_extractor.cpp +++ b/src/dwarfs/filesystem_extractor.cpp @@ -364,9 +364,10 @@ bool filesystem_extractor_::extract( ::archive_entry_copy_stat(ae, &st); if (inode.is_symlink()) { - std::string link; - if (fs.readlink(inode, &link) != 0) { - LOG_ERROR << "readlink() failed"; + std::error_code ec; + auto link = fs.readlink(inode, ec); + if (ec) { + LOG_ERROR << "readlink() failed: " << ec.message(); } if (opts.progress) { bytes_written += link.size(); diff --git a/src/dwarfs/filesystem_v2.cpp b/src/dwarfs/filesystem_v2.cpp index b8fb3828..a9deb00d 100644 --- a/src/dwarfs/filesystem_v2.cpp +++ b/src/dwarfs/filesystem_v2.cpp @@ -75,6 +75,28 @@ void check_section_logger(logger& lgr, fs_section const& section) { } } +template +auto call_ec_throw(Fn&& fn) { + std::error_code ec; + auto result = std::forward(fn)(ec); + if (ec) { + throw std::system_error(ec); + } + return result; +} + +template +R call_int_error(Fn&& fn, std::error_code& ec) { + R res; + auto err = std::forward(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 - readlink(inode_view entry, readlink_mode mode) const override; + std::string readlink(inode_view entry, readlink_mode mode, + 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_::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_::readlink(inode_view entry, std::string* buf, } template -folly::Expected -filesystem_::readlink(inode_view entry, - readlink_mode mode) const { - PERFMON_CLS_SCOPED_SECTION(readlink_expected) - return meta_.readlink(entry, mode); +std::string +filesystem_::readlink_ec(inode_view entry, readlink_mode mode, + std::error_code& ec) const { + return call_int_error( + [&](auto& buf) { return meta_.readlink(entry, &buf, mode); }, ec); +} + +template +std::string +filesystem_::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 +std::string filesystem_::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 diff --git a/src/dwarfs/metadata_v2.cpp b/src/dwarfs/metadata_v2.cpp index 438f4a35..07ee90a9 100644 --- a/src/dwarfs/metadata_v2.cpp +++ b/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 - readlink(inode_view iv, readlink_mode mode) const override; - int statvfs(vfs_stat* stbuf) const override; std::optional get_chunks(int inode) const override; @@ -1695,16 +1692,6 @@ int metadata_::readlink(inode_view iv, std::string* buf, return -EINVAL; } -template -folly::Expected -metadata_::readlink(inode_view iv, readlink_mode mode) const { - if (iv.is_symlink()) { - return link_value(iv, mode); - } - - return folly::makeUnexpected(-EINVAL); -} - template int metadata_::statvfs(vfs_stat* stbuf) const { ::memset(stbuf, 0, sizeof(*stbuf)); diff --git a/src/dwarfsck_main.cpp b/src/dwarfsck_main.cpp index e07ff00c..992a24da 100644 --- a/src/dwarfsck_main.cpp +++ b/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; } diff --git a/test/filesystem_test.cpp b/test/filesystem_test.cpp index 44570460..600ced74 100644 --- a/test/filesystem_test.cpp +++ b/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); - EXPECT_TRUE(r); - EXPECT_EQ("subdir/test.txt", *r); + std::error_code ec; + auto r = fs.readlink(*i1, readlink_mode::unix, ec); + EXPECT_FALSE(ec); + EXPECT_EQ("subdir/test.txt", r); } { - auto r = fs.readlink(*i3); - EXPECT_FALSE(r); - EXPECT_EQ(-EINVAL, r.error()); + std::error_code ec; + auto r = fs.readlink(*i3, ec); + 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()); } } @@ -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); - EXPECT_TRUE(r); - EXPECT_EQ("subdir/test.txt", *r); + std::error_code ec; + auto r = fs.readlink(*i1, readlink_mode::unix, ec); + EXPECT_FALSE(ec); + EXPECT_EQ("subdir/test.txt", r); } { - auto r = fs.readlink(*i3); - EXPECT_FALSE(r); - EXPECT_EQ(-EINVAL, r.error()); + std::error_code ec; + auto r = fs.readlink(*i3, ec); + EXPECT_TRUE(ec); + EXPECT_EQ(EINVAL, ec.value()); } } diff --git a/test/tool_main_test.cpp b/test/tool_main_test.cpp index 3728f6b9..77b3f11c 100644 --- a/test/tool_main_test.cpp +++ b/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\])"