From c371070580afbb7f18972ed52e973f581496f881 Mon Sep 17 00:00:00 2001 From: John Ericson Date: Tue, 7 May 2024 00:14:49 -0400 Subject: [PATCH] Use `std::filesystem` functions in more places This makes for shorter and more portable code. The only tricky part is catching exceptions: I just searched for near by `catch (Error &)` or `catch (SysError &)` and adjusted them to `catch (std::filesystem::filesystem_error &)` according to my human judgement. Good for windows portability; will help @siddhantk232 with his GSOC project. --- configure.ac | 1 - src/libcmd/repl.cc | 1 + src/libstore/builtins/buildenv.cc | 4 +- src/libstore/globals.cc | 4 +- src/libstore/profiles.cc | 2 + src/libstore/unix/gc.cc | 14 ++-- src/libstore/unix/local-store.hh | 2 +- src/libutil/file-system.cc | 83 ++++++------------- src/libutil/file-system.hh | 19 +---- src/libutil/linux/cgroup.cc | 2 +- src/libutil/posix-source-accessor.cc | 14 ++-- src/libutil/unix/file-descriptor.cc | 1 + .../nix-collect-garbage.cc | 10 +-- src/nix/config-check.cc | 3 +- 14 files changed, 61 insertions(+), 99 deletions(-) diff --git a/configure.ac b/configure.ac index 8f60bf4be..b2a5794b5 100644 --- a/configure.ac +++ b/configure.ac @@ -63,7 +63,6 @@ AC_SYS_LARGEFILE # Solaris-specific stuff. -AC_STRUCT_DIRENT_D_TYPE case "$host_os" in solaris*) # Solaris requires -lsocket -lnsl for network functions diff --git a/src/libcmd/repl.cc b/src/libcmd/repl.cc index bade1b538..d2f2a9648 100644 --- a/src/libcmd/repl.cc +++ b/src/libcmd/repl.cc @@ -264,6 +264,7 @@ StringSet NixRepl::completePrefix(const std::string & prefix) completions.insert(prev + dir + "/" + entry.name); } } catch (Error &) { + } catch (std::filesystem::filesystem_error &) { } } else if ((dot = cur.rfind('.')) == std::string::npos) { /* This is a variable name; look it up in the current scope. */ diff --git a/src/libstore/builtins/buildenv.cc b/src/libstore/builtins/buildenv.cc index e009f5b9d..ebd8f1348 100644 --- a/src/libstore/builtins/buildenv.cc +++ b/src/libstore/builtins/buildenv.cc @@ -21,8 +21,8 @@ static void createLinks(State & state, const Path & srcDir, const Path & dstDir, try { srcFiles = readDirectory(srcDir); - } catch (SysError & e) { - if (e.errNo == ENOTDIR) { + } catch (std::filesystem::filesystem_error & e) { + if (e.code() == std::errc::not_a_directory) { warn("not including '%s' in the user environment because it's not a directory", srcDir); return; } diff --git a/src/libstore/globals.cc b/src/libstore/globals.cc index dfe3044ea..f0096b981 100644 --- a/src/libstore/globals.cc +++ b/src/libstore/globals.cc @@ -348,8 +348,8 @@ void initPlugins() auto ents = readDirectory(pluginFile); for (const auto & ent : ents) pluginFiles.emplace_back(pluginFile + "/" + ent.name); - } catch (SysError & e) { - if (e.errNo != ENOTDIR) + } catch (std::filesystem::filesystem_error & e) { + if (e.code() != std::errc::not_a_directory) throw; pluginFiles.emplace_back(pluginFile); } diff --git a/src/libstore/profiles.cc b/src/libstore/profiles.cc index 73d3976f4..a0bb60410 100644 --- a/src/libstore/profiles.cc +++ b/src/libstore/profiles.cc @@ -338,6 +338,8 @@ Path getDefaultProfile() return absPath(readLink(profileLink), dirOf(profileLink)); } catch (Error &) { return profileLink; + } catch (std::filesystem::filesystem_error &) { + return profileLink; } } diff --git a/src/libstore/unix/gc.cc b/src/libstore/unix/gc.cc index be5794395..6677946aa 100644 --- a/src/libstore/unix/gc.cc +++ b/src/libstore/unix/gc.cc @@ -203,7 +203,7 @@ void LocalStore::findTempRoots(Roots & tempRoots, bool censor) } -void LocalStore::findRoots(const Path & path, unsigned char type, Roots & roots) +void LocalStore::findRoots(const Path & path, std::filesystem::file_type type, Roots & roots) { auto foundRoot = [&](const Path & path, const Path & target) { try { @@ -217,15 +217,15 @@ void LocalStore::findRoots(const Path & path, unsigned char type, Roots & roots) try { - if (type == DT_UNKNOWN) + if (type == std::filesystem::file_type::unknown) type = getFileType(path); - if (type == DT_DIR) { + if (type == std::filesystem::file_type::directory) { for (auto & i : readDirectory(path)) findRoots(path + "/" + i.name, i.type, roots); } - else if (type == DT_LNK) { + else if (type == std::filesystem::file_type::symlink) { Path target = readLink(path); if (isInStore(target)) foundRoot(path, target); @@ -247,7 +247,7 @@ void LocalStore::findRoots(const Path & path, unsigned char type, Roots & roots) } } - else if (type == DT_REG) { + else if (type == std::filesystem::file_type::regular) { auto storePath = maybeParseStorePath(storeDir + "/" + std::string(baseNameOf(path))); if (storePath && isValidPath(*storePath)) roots[std::move(*storePath)].emplace(path); @@ -268,8 +268,8 @@ void LocalStore::findRoots(const Path & path, unsigned char type, Roots & roots) void LocalStore::findRootsNoTemp(Roots & roots, bool censor) { /* Process direct roots in {gcroots,profiles}. */ - findRoots(stateDir + "/" + gcRootsDir, DT_UNKNOWN, roots); - findRoots(stateDir + "/profiles", DT_UNKNOWN, roots); + findRoots(stateDir + "/" + gcRootsDir, std::filesystem::file_type::unknown, roots); + findRoots(stateDir + "/profiles", std::filesystem::file_type::unknown, roots); /* Add additional roots returned by different platforms-specific heuristics. This is typically used to add running programs to diff --git a/src/libstore/unix/local-store.hh b/src/libstore/unix/local-store.hh index 47d3c04bc..15bcc826f 100644 --- a/src/libstore/unix/local-store.hh +++ b/src/libstore/unix/local-store.hh @@ -371,7 +371,7 @@ private: PathSet queryValidPathsOld(); ValidPathInfo queryPathInfoOld(const Path & path); - void findRoots(const Path & path, unsigned char type, Roots & roots); + void findRoots(const Path & path, std::filesystem::file_type type, Roots & roots); void findRootsNoTemp(Roots & roots, bool censor); diff --git a/src/libutil/file-system.cc b/src/libutil/file-system.cc index b03bb767b..0e68241fc 100644 --- a/src/libutil/file-system.cc +++ b/src/libutil/file-system.cc @@ -120,10 +120,10 @@ Path canonPath(PathView path, bool resolveSymlinks) Path dirOf(const PathView path) { - Path::size_type pos = path.rfind('/'); + Path::size_type pos = NativePathTrait::rfindPathSep(path); if (pos == path.npos) return "."; - return pos == 0 ? "/" : Path(path, 0, pos); + return fs::path{path}.parent_path().string(); } @@ -217,72 +217,36 @@ bool pathAccessible(const Path & path) Path readLink(const Path & path) { -#ifndef _WIN32 checkInterrupt(); - std::vector buf; - for (ssize_t bufSize = PATH_MAX/4; true; bufSize += bufSize/2) { - buf.resize(bufSize); - ssize_t rlSize = readlink(path.c_str(), buf.data(), bufSize); - if (rlSize == -1) - if (errno == EINVAL) - throw Error("'%1%' is not a symlink", path); - else - throw SysError("reading symbolic link '%1%'", path); - else if (rlSize < bufSize) - return std::string(buf.data(), rlSize); - } -#else - // TODO modern Windows does in fact support symlinks - throw UnimplementedError("reading symbolic link '%1%'", path); -#endif + return fs::read_symlink(path).string(); } bool isLink(const Path & path) { - return getFileType(path) == DT_LNK; + return getFileType(path) == fs::file_type::symlink; } -DirEntries readDirectory(DIR *dir, const Path & path) +DirEntries readDirectory(const Path & path) { DirEntries entries; entries.reserve(64); - struct dirent * dirent; - while (errno = 0, dirent = readdir(dir)) { /* sic */ + for (auto & entry : fs::directory_iterator{path}) { checkInterrupt(); - std::string name = dirent->d_name; - if (name == "." || name == "..") continue; - entries.emplace_back(name, dirent->d_ino, -#ifdef HAVE_STRUCT_DIRENT_D_TYPE - dirent->d_type -#else - DT_UNKNOWN -#endif - ); + entries.emplace_back( + entry.path().filename().string(), + entry.symlink_status().type()); } - if (errno) throw SysError("reading directory '%1%'", path); return entries; } -DirEntries readDirectory(const Path & path) + +fs::file_type getFileType(const Path & path) { - AutoCloseDir dir(opendir(path.c_str())); - if (!dir) throw SysError("opening directory '%1%'", path); - - return readDirectory(dir.get(), path); -} - - -unsigned char getFileType(const Path & path) -{ - struct stat st = lstat(path); - if (S_ISDIR(st.st_mode)) return DT_DIR; - if (S_ISLNK(st.st_mode)) return DT_LNK; - if (S_ISREG(st.st_mode)) return DT_REG; - return DT_UNKNOWN; + return fs::symlink_status(path).type(); } @@ -432,8 +396,15 @@ static void _deletePath(Descriptor parentfd, const Path & path, uint64_t & bytes AutoCloseDir dir(fdopendir(fd)); if (!dir) throw SysError("opening directory '%1%'", path); - for (auto & i : readDirectory(dir.get(), path)) - _deletePath(dirfd(dir.get()), path + "/" + i.name, bytesFreed); + + struct dirent * dirent; + while (errno = 0, dirent = readdir(dir.get())) { /* sic */ + checkInterrupt(); + std::string childName = dirent->d_name; + if (childName == "." || childName == "..") continue; + _deletePath(dirfd(dir.get()), path + "/" + childName, bytesFreed); + } + if (errno) throw SysError("reading directory '%1%'", path); } int flags = S_ISDIR(st.st_mode) ? AT_REMOVEDIR : 0; @@ -611,13 +582,7 @@ std::pair createTempFile(const Path & prefix) void createSymlink(const Path & target, const Path & link) { -#ifndef _WIN32 - if (symlink(target.c_str(), link.c_str())) - throw SysError("creating symlink from '%1%' to '%2%'", link, target); -#else - // TODO modern Windows does in fact support symlinks - throw UnimplementedError("createSymlink"); -#endif + fs::create_symlink(target, link); } void replaceSymlink(const Path & target, const Path & link) @@ -627,8 +592,8 @@ void replaceSymlink(const Path & target, const Path & link) try { createSymlink(target, tmp); - } catch (SysError & e) { - if (e.errNo == EEXIST) continue; + } catch (fs::filesystem_error & e) { + if (e.code() == std::errc::file_exists) continue; throw; } diff --git a/src/libutil/file-system.hh b/src/libutil/file-system.hh index 0c4e7cfdd..2ecf2881c 100644 --- a/src/libutil/file-system.hh +++ b/src/libutil/file-system.hh @@ -27,13 +27,6 @@ #include #include -#ifndef HAVE_STRUCT_DIRENT_D_TYPE -#define DT_UNKNOWN 0 -#define DT_REG 1 -#define DT_LNK 2 -#define DT_DIR 3 -#endif - /** * Polyfill for MinGW * @@ -132,20 +125,16 @@ bool isLink(const Path & path); struct DirEntry { std::string name; - ino_t ino; - /** - * one of DT_* - */ - unsigned char type; - DirEntry(std::string name, ino_t ino, unsigned char type) - : name(std::move(name)), ino(ino), type(type) { } + std::filesystem::file_type type; + DirEntry(std::string name, std::filesystem::file_type type) + : name(std::move(name)), type(type) { } }; typedef std::vector DirEntries; DirEntries readDirectory(const Path & path); -unsigned char getFileType(const Path & path); +std::filesystem::file_type getFileType(const Path & path); /** * Read the contents of a file into a string. diff --git a/src/libutil/linux/cgroup.cc b/src/libutil/linux/cgroup.cc index 8b8942643..9234a4b2e 100644 --- a/src/libutil/linux/cgroup.cc +++ b/src/libutil/linux/cgroup.cc @@ -65,7 +65,7 @@ static CgroupStats destroyCgroup(const Path & cgroup, bool returnStats) /* Otherwise, manually kill every process in the subcgroups and this cgroup. */ for (auto & entry : readDirectory(cgroup)) { - if (entry.type != DT_DIR) continue; + if (entry.type != std::filesystem::file_type::directory) continue; destroyCgroup(cgroup + "/" + entry.name, false); } diff --git a/src/libutil/posix-source-accessor.cc b/src/libutil/posix-source-accessor.cc index e9c554939..a2559a0fb 100644 --- a/src/libutil/posix-source-accessor.cc +++ b/src/libutil/posix-source-accessor.cc @@ -134,13 +134,17 @@ SourceAccessor::DirEntries PosixSourceAccessor::readDirectory(const CanonPath & DirEntries res; for (auto & entry : nix::readDirectory(makeAbsPath(path).string())) { std::optional type; + // cannot exhaustively enumerate because implementation-specific + // additional file types are allowed. +#pragma GCC diagnostic push +#pragma GCC diagnostic ignored "-Wswitch-enum" switch (entry.type) { - case DT_REG: type = Type::tRegular; break; - #ifndef _WIN32 - case DT_LNK: type = Type::tSymlink; break; - #endif - case DT_DIR: type = Type::tDirectory; break; + case std::filesystem::file_type::regular: type = Type::tRegular; break; + case std::filesystem::file_type::symlink: type = Type::tSymlink; break; + case std::filesystem::file_type::directory: type = Type::tDirectory; break; + default: type = tMisc; } +#pragma GCC diagnostic pop res.emplace(entry.name, type); } return res; diff --git a/src/libutil/unix/file-descriptor.cc b/src/libutil/unix/file-descriptor.cc index 27c8d821b..068b37843 100644 --- a/src/libutil/unix/file-descriptor.cc +++ b/src/libutil/unix/file-descriptor.cc @@ -133,6 +133,7 @@ void closeMostFDs(const std::set & exceptions) } return; } catch (SysError &) { + } catch (std::filesystem::filesystem_error &) { } #endif diff --git a/src/nix-collect-garbage/nix-collect-garbage.cc b/src/nix-collect-garbage/nix-collect-garbage.cc index bb3f1bc6a..9dfbea3f1 100644 --- a/src/nix-collect-garbage/nix-collect-garbage.cc +++ b/src/nix-collect-garbage/nix-collect-garbage.cc @@ -31,14 +31,14 @@ void removeOldGenerations(std::string dir) checkInterrupt(); auto path = dir + "/" + i.name; - auto type = i.type == DT_UNKNOWN ? getFileType(path) : i.type; + auto type = i.type == std::filesystem::file_type::unknown ? getFileType(path) : i.type; - if (type == DT_LNK && canWrite) { + if (type == std::filesystem::file_type::symlink && canWrite) { std::string link; try { link = readLink(path); - } catch (SysError & e) { - if (e.errNo == ENOENT) continue; + } catch (std::filesystem::filesystem_error & e) { + if (e.code() == std::errc::no_such_file_or_directory) continue; throw; } if (link.find("link") != std::string::npos) { @@ -49,7 +49,7 @@ void removeOldGenerations(std::string dir) } else deleteOldGenerations(path, dryRun); } - } else if (type == DT_DIR) { + } else if (type == std::filesystem::file_type::directory) { removeOldGenerations(path); } } diff --git a/src/nix/config-check.cc b/src/nix/config-check.cc index f7c4cebec..f23e36fb5 100644 --- a/src/nix/config-check.cc +++ b/src/nix/config-check.cc @@ -107,7 +107,8 @@ struct CmdConfigCheck : StoreCommand if (profileDir.find("/profiles/") == std::string::npos) dirs.insert(dir); } - } catch (SystemError &) {} + } catch (SystemError &) { + } catch (std::filesystem::filesystem_error &) {} } if (!dirs.empty()) {