From 31f07c69ab50373dfab170360ee570b882703abc Mon Sep 17 00:00:00 2001 From: Benjamin Barenblat Date: Thu, 18 Feb 2016 23:34:10 -0500 Subject: Create and use RAII directory abstraction --- CMakeLists.txt | 2 +- src/operations.cc | 66 ++++++++++++++++++++++--------------------------- src/posix_extras.cc | 71 +++++++++++++++++++++++++++++++++++++++++++++++++++++ src/posix_extras.h | 40 ++++++++++++++++++++++++++++-- 4 files changed, 140 insertions(+), 39 deletions(-) diff --git a/CMakeLists.txt b/CMakeLists.txt index 8ff0ac8..16210b5 100644 --- a/CMakeLists.txt +++ b/CMakeLists.txt @@ -24,7 +24,7 @@ project( set(CMAKE_CXX_COMPILER clang++) set(CMAKE_CXX_FLAGS "${CMAKE_CXX_FLAGS} -D_FORTIFY_SOURCE=2") -set(CMAKE_CXX_FLAGS "${CMAKE_CXX_FLAGS} -std=c++11") +set(CMAKE_CXX_FLAGS "${CMAKE_CXX_FLAGS} -std=c++14") set(CMAKE_CXX_FLAGS "${CMAKE_CXX_FLAGS} -ftrapv") set(CMAKE_CXX_FLAGS "${CMAKE_CXX_FLAGS} -fstack-protector-strong --param=ssp-buffer-size=4") set(CMAKE_CXX_FLAGS "${CMAKE_CXX_FLAGS} -Weverything -Wno-c++98-compat") diff --git a/src/operations.cc b/src/operations.cc index e709b8a..86cd880 100644 --- a/src/operations.cc +++ b/src/operations.cc @@ -22,8 +22,10 @@ #include #include +#include #include #include +#include #define FUSE_USE_VERSION 26 #include @@ -42,12 +44,6 @@ namespace { // Pointer to the directory underlying the mount point. File* root_; -struct Directory { - DIR* fd; - dirent* entry; - off_t offset; -}; - Directory* FileInfoDirectory(fuse_file_info* const file_info) { return reinterpret_cast(file_info->fh); } @@ -108,18 +104,22 @@ int Opendir(const char* const path, fuse_file_info* const file_info) { LOG(INFO) << "opendir(" << path << ")"; std::unique_ptr directory; + try { - directory.reset(new Directory); - } catch (std::bad_alloc) { + directory.reset(new Directory( + strcmp(path, "/") == 0 + ? + // They're asking to open the mount point. + *root_ + : + // Trim the leading slash so OpenAt will treat it relative to root_. + root_->OpenAt(path + 1, O_DIRECTORY))); + } catch (const std::bad_alloc&) { return -ENOMEM; + } catch (const IoError& e) { + return -e.number(); } - if ((directory->fd = opendir(path)) == nullptr) { - return -errno; - } - directory->entry = nullptr; - directory->offset = 0; - static_assert(sizeof(file_info->fh) == sizeof(uintptr_t), "FUSE file handles are a different size than pointers"); file_info->fh = reinterpret_cast(directory.release()); @@ -132,31 +132,26 @@ int Readdir(const char*, void* const buffer, fuse_fill_dir_t filler, Directory* const directory = FileInfoDirectory(file_info); - if (offset != directory->offset) { - seekdir(directory->fd, offset); - directory->entry = nullptr; - directory->offset = offset; - } + try { + static_assert(std::is_same(), + "off_t is not convertible with long"); + if (offset != directory->offset()) { + directory->Seek(offset); + } - while (true) { - if (!directory->entry) { - directory->entry = readdir(directory->fd); - if (!directory->entry) { + for (std::experimental::optional entry = directory->ReadOne(); + entry; entry = directory->ReadOne()) { + struct stat stats; + std::memset(&stats, 0, sizeof(stats)); + stats.st_ino = entry->d_ino; + stats.st_mode = DirectoryTypeToFileType(entry->d_type); + const off_t next_offset = directory->offset(); + if (filler(buffer, entry->d_name, &stats, next_offset)) { break; } } - - struct stat stats; - std::memset(&stats, 0, sizeof(stats)); - stats.st_ino = directory->entry->d_ino; - stats.st_mode = DirectoryTypeToFileType(directory->entry->d_type); - const off_t next_offset = telldir(directory->fd); - if (filler(buffer, directory->entry->d_name, &stats, next_offset)) { - break; - } - - directory->entry = nullptr; - directory->offset = next_offset; + } catch (const IoError& e) { + return -e.number(); } return 0; @@ -165,7 +160,6 @@ int Readdir(const char*, void* const buffer, fuse_fill_dir_t filler, int Releasedir(const char*, fuse_file_info* const file_info) { LOG(INFO) << "releasedir"; Directory* const directory = FileInfoDirectory(file_info); - closedir(directory->fd); delete directory; return 0; } diff --git a/src/posix_extras.cc b/src/posix_extras.cc index c3c841c..0acae13 100644 --- a/src/posix_extras.cc +++ b/src/posix_extras.cc @@ -12,15 +12,18 @@ // License for the specific language governing permissions and limitations under // the License. +#define _BSD_SOURCE 1 #define _POSIX_C_SOURCE 201502L #undef _GNU_SOURCE #include "posix_extras.h" #include +#include #include #include +#include #include #include #include // a POSIX header, not a libc one @@ -52,6 +55,11 @@ File::File(const char* const path, const int flags) : path_(path) { if ((fd_ = open(path, flags)) == -1) { throw IoError(); } + VLOG(1) << "opening file descriptor " << fd_; +} + +File::File(const File& other) : path_(other.path_), fd_(other.Duplicate()) { + VLOG(1) << "opening file descriptor " << fd_; } File::~File() noexcept { @@ -82,4 +90,67 @@ struct stat File::LinkStatAt(const char* const path) const { return result; } +File File::OpenAt(const char* const path, const int flags) const { + if (path[0] == '/') { + throw std::invalid_argument("absolute path"); + } + + File result; + if ((result.fd_ = openat(fd_, path, flags)) == -1) { + throw IoError(); + } + result.path_ = path_ + "/" + path; + return result; +} + +int File::Duplicate() const { + int result; + if ((result = dup(fd_)) == -1) { + throw IoError(); + } + return result; +} + +Directory::Directory(const File& file) { + // "After a successful call to fdopendir(), fd is used internally by the + // implementation, and should not otherwise be used by the application." We + // therefore need to grab an unmanaged copy of the file descriptor from file. + if (!(stream_ = fdopendir(file.Duplicate()))) { + throw IoError(); + } + rewinddir(stream_); +} + +Directory::~Directory() noexcept { + if (closedir(stream_) == -1) { + LOG(ERROR) << "failed to close directory stream: " + << IoError::Message(errno); + } +} + +long Directory::offset() const { + long result; + if ((result = telldir(stream_)) == -1) { + throw IoError(); + } + return result; +} + +void Directory::Seek(const long offset) noexcept { + seekdir(stream_, offset); +} + +std::experimental::optional Directory::ReadOne() { + dirent* result; + errno = 0; + if (!(result = readdir(stream_))) { + if (errno == 0) { + return std::experimental::nullopt; + } else { + throw IoError(); + } + } + return *result; +} + } // scoville diff --git a/src/posix_extras.h b/src/posix_extras.h index 823357c..9498249 100644 --- a/src/posix_extras.h +++ b/src/posix_extras.h @@ -16,9 +16,11 @@ #define POSIX_EXTRAS_H_ #include +#include #include #include +#include #include #include #include @@ -42,10 +44,36 @@ class IoError : public std::runtime_error { int number_; }; +class File; + +// RAII wrapper for Unix directory streams. +class Directory { + public: + explicit Directory(const File&); + virtual ~Directory() noexcept; + + long offset() const; + + void Seek(long) noexcept; + + std::experimental::optional ReadOne(); + + private: + Directory(const Directory&) = delete; + Directory(Directory&&) = delete; + + void operator=(const Directory&) = delete; + void operator=(Directory&&) = delete; + + DIR* stream_; +}; + // RAII wrapper for Unix file descriptors. class File { public: File(const char* path, int flags); + File(const File&); + File(File&& other) = default; virtual ~File() noexcept; const std::string& path() const noexcept { return path_; } @@ -57,15 +85,23 @@ class File { // indeed be relative (i.e., it must not start with '/'). struct stat LinkStatAt(const char* path) const; + // Calls openat(2) on the path relative to the file descriptor. The path must + // indeed be relative (i.e., it must not start with '/'). + File OpenAt(const char* path, int flags) const; + private: - File(const File&) = delete; - File(File&&) = delete; + File() {} void operator=(const File&) = delete; void operator=(File&&) = delete; + // Duplicates fd_ and returns the raw new file descriptor. + int Duplicate() const; + std::string path_; int fd_; + + friend Directory::Directory(const File&); }; } // scoville -- cgit v1.2.3