h/fish-shell
1
Fork 0
mirror of https://github.com/fish-shell/fish-shell.git synced 2026-06-29 06:01:16 -03:00
Code Issues Packages Projects Releases Wiki Activity

dir_iter_t to use unique_ptr for closing directory

Browse Source 
dir_iter_t closes its DIR* member in two places: the move assignment and
the destructor. Simplify this by closing it in the destructor of the DIR*
member which is called in both places. Use std::unique_ptr, which is shorter
than a dedicated wrapper class. Conveniently, it calls the deleter only if
the pointer is not-null.  Unfortunately, std::unique_ptr requires explicit
conversion to DIR* when interacting with C APIs but it's probably still
better than a wrapper class.

This means that the noncopyable_t annotation is now implied due to the
unique_ptr member.
Additionally, we could probably remove the user-declared move constructor
and move assignment (the compiler-generated ones should be good enough). To
be safe, keep them around since they also erase the fd (though I hope we
don't rely on that behavior anywhere).

We should perhaps remove the user-declared destructor entirely but
dir_iter_t::entry_t also has one, I'm not sure why. Maybe there's a good
reason, like code size.

No functional change.
This commit is contained in:
Johannes Altmanninger
2022-10-07 19:28:55 -05:00
parent a99f588328
commit da5d93b4de
2 changed files with 12 additions and 16 deletions
Download Patch File Download Diff File Expand all files Collapse all files

21
src/wutil.cpp
View File

@@ -173,17 +173,17 @@ void dir_iter_t::entry_t::do_stat() const {
}
dir_iter_t::dir_iter_t(const wcstring &path) {
dir_ = wopendir(path);
dir_.reset(wopendir(path));
if (!dir_) {
error_ = errno;
return;
}
entry_.dirfd_ = dirfd(dir_);
entry_.dirfd_ = dirfd(&*dir_);
}
dir_iter_t::dir_iter_t(dir_iter_t &&rhs) {
// Steal the fields; ensure rhs no longer has FILE* and forgets its fd.
this->dir_ = rhs.dir_;
this->dir_ = std::move(rhs.dir_);
this->error_ = rhs.error_;
this->entry_ = std::move(rhs.entry_);
rhs.dir_ = nullptr;
@@ -191,10 +191,7 @@ dir_iter_t::dir_iter_t(dir_iter_t &&rhs) {
}
dir_iter_t &dir_iter_t::operator=(dir_iter_t &&rhs) {
if (this->dir_) {
(void)closedir(this->dir_);
}
this->dir_ = rhs.dir_;
this->dir_ = std::move(rhs.dir_);
this->error_ = rhs.error_;
this->entry_ = std::move(rhs.entry_);
rhs.dir_ = nullptr;
@@ -204,22 +201,18 @@ dir_iter_t &dir_iter_t::operator=(dir_iter_t &&rhs) {
void dir_iter_t::rewind() {
if (dir_) {
rewinddir(dir_);
rewinddir(&*dir_);
}
}
dir_iter_t::~dir_iter_t() {
if (dir_) {
(void)closedir(dir_);
}
}
dir_iter_t::~dir_iter_t() = default;
const dir_iter_t::entry_t *dir_iter_t::next() {
if (!dir_) {
return nullptr;
}
errno = 0;
struct dirent *dent = readdir(dir_);
struct dirent *dent = readdir(&*dir_);
if (!dent) {
error_ = errno;
return nullptr;

7
src/wutil.h
View File

@@ -190,7 +190,7 @@ class dir_iter_t : noncopyable_t {
bool valid() const { return dir_ != nullptr; }
/// \return the underlying file descriptor, or -1 if invalid.
int fd() const { return dir_ ? dirfd(dir_) : -1; }
int fd() const { return dir_ ? dirfd(&*dir_) : -1; }
/// Rewind the directory to the beginning.
void rewind();
@@ -247,7 +247,10 @@ class dir_iter_t : noncopyable_t {
};
private:
DIR *dir_{nullptr};
struct dir_closer_t {
void operator()(DIR *dir) const { (void)closedir(dir); }
};
std::unique_ptr<DIR, dir_closer_t> dir_{nullptr};
int error_{0};
entry_t entry_;
};