From c2de0a232c1cfddb1f1385ffd23dd43a2099458e Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Th=C3=A9ophane=20Hufschmitt?= Date: Thu, 17 Mar 2022 15:28:46 +0100 Subject: [PATCH] =?UTF-8?q?Create=20a=20wrapper=20around=20stdlib=E2=80=99?= =?UTF-8?q?s=20`rename`?= MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit Directly takes some c++ strings, and gently throws an exception on error (rather than having to inline this logic everywhere) --- src/libstore/build/derivation-goal.cc | 3 +-- src/libstore/build/local-derivation-goal.cc | 8 +++----- src/libstore/builtins/unpack-channel.cc | 3 +-- src/libstore/gc.cc | 4 +--- src/libstore/local-binary-cache-store.cc | 3 +-- src/libstore/local-store.cc | 6 ++---- src/libstore/optimise-store.cc | 6 ++++-- src/libutil/filesystem.cc | 9 +++++++-- src/libutil/util.hh | 2 ++ 9 files changed, 22 insertions(+), 22 deletions(-) diff --git a/src/libstore/build/derivation-goal.cc b/src/libstore/build/derivation-goal.cc index 3fff2385f..93f923f18 100644 --- a/src/libstore/build/derivation-goal.cc +++ b/src/libstore/build/derivation-goal.cc @@ -705,8 +705,7 @@ static void movePath(const Path & src, const Path & dst) if (changePerm) chmod_(src, st.st_mode | S_IWUSR); - if (rename(src.c_str(), dst.c_str())) - throw SysError("renaming '%1%' to '%2%'", src, dst); + moveFile(src, dst); if (changePerm) chmod_(dst, st.st_mode); diff --git a/src/libstore/build/local-derivation-goal.cc b/src/libstore/build/local-derivation-goal.cc index 79a241ae0..2435377e2 100644 --- a/src/libstore/build/local-derivation-goal.cc +++ b/src/libstore/build/local-derivation-goal.cc @@ -223,8 +223,7 @@ static void movePath(const Path & src, const Path & dst) if (changePerm) chmod_(src, st.st_mode | S_IWUSR); - if (rename(src.c_str(), dst.c_str())) - throw SysError("renaming '%1%' to '%2%'", src, dst); + moveFile(src, dst); if (changePerm) chmod_(dst, st.st_mode); @@ -311,7 +310,7 @@ bool LocalDerivationGoal::cleanupDecideWhetherDiskFull() if (buildMode != bmCheck && status.known->isValid()) continue; auto p = worker.store.printStorePath(status.known->path); if (pathExists(chrootRootDir + p)) - rename((chrootRootDir + p).c_str(), p.c_str()); + moveFile((chrootRootDir + p), p); } return diskFull; @@ -2625,8 +2624,7 @@ DrvOutputs LocalDerivationGoal::registerOutputs() Path prev = path + checkSuffix; deletePath(prev); Path dst = path + checkSuffix; - if (rename(path.c_str(), dst.c_str())) - throw SysError("renaming '%s' to '%s'", path, dst); + moveFile(path, dst); } } diff --git a/src/libstore/builtins/unpack-channel.cc b/src/libstore/builtins/unpack-channel.cc index 426d58a53..a8417d7ff 100644 --- a/src/libstore/builtins/unpack-channel.cc +++ b/src/libstore/builtins/unpack-channel.cc @@ -22,8 +22,7 @@ void builtinUnpackChannel(const BasicDerivation & drv) auto entries = readDirectory(out); if (entries.size() != 1) throw Error("channel tarball '%s' contains more than one file", src); - if (rename((out + "/" + entries[0].name).c_str(), (out + "/" + channelName).c_str()) == -1) - throw SysError("renaming channel directory"); + moveFile((out + "/" + entries[0].name), (out + "/" + channelName)); } } diff --git a/src/libstore/gc.cc b/src/libstore/gc.cc index d58ed78b1..3682f81cc 100644 --- a/src/libstore/gc.cc +++ b/src/libstore/gc.cc @@ -39,9 +39,7 @@ static void makeSymlink(const Path & link, const Path & target) createSymlink(target, tempLink); /* Atomically replace the old one. */ - if (rename(tempLink.c_str(), link.c_str()) == -1) - throw SysError("cannot rename '%1%' to '%2%'", - tempLink , link); + moveFile(tempLink, link); } diff --git a/src/libstore/local-binary-cache-store.cc b/src/libstore/local-binary-cache-store.cc index ba4416f6d..ff7403f9d 100644 --- a/src/libstore/local-binary-cache-store.cc +++ b/src/libstore/local-binary-cache-store.cc @@ -57,8 +57,7 @@ protected: AutoDelete del(tmp, false); StreamToSourceAdapter source(istream); writeFile(tmp, source); - if (rename(tmp.c_str(), path2.c_str())) - throw SysError("renaming '%1%' to '%2%'", tmp, path2); + moveFile(tmp, path2); del.cancel(); } diff --git a/src/libstore/local-store.cc b/src/libstore/local-store.cc index eba3b0fa5..7f708b243 100644 --- a/src/libstore/local-store.cc +++ b/src/libstore/local-store.cc @@ -1430,8 +1430,7 @@ StorePath LocalStore::addToStoreFromDump(Source & source0, std::string_view name writeFile(realPath, dumpSource); } else { /* Move the temporary path we restored above. */ - if (rename(tempPath.c_str(), realPath.c_str())) - throw Error("renaming '%s' to '%s'", tempPath, realPath); + moveFile(tempPath, realPath); } /* For computing the nar hash. In recursive SHA-256 mode, this @@ -1942,8 +1941,7 @@ void LocalStore::addBuildLog(const StorePath & drvPath, std::string_view log) writeFile(tmpFile, compress("bzip2", log)); - if (rename(tmpFile.c_str(), logPath.c_str()) != 0) - throw SysError("renaming '%1%' to '%2%'", tmpFile, logPath); + moveFile(tmpFile, logPath); } std::optional LocalStore::getVersion() diff --git a/src/libstore/optimise-store.cc b/src/libstore/optimise-store.cc index 8af9b1dde..20b9c7611 100644 --- a/src/libstore/optimise-store.cc +++ b/src/libstore/optimise-store.cc @@ -229,7 +229,9 @@ void LocalStore::optimisePath_(Activity * act, OptimiseStats & stats, } /* Atomically replace the old file with the new hard link. */ - if (rename(tempLink.c_str(), path.c_str()) == -1) { + try { + moveFile(tempLink, path); + } catch (SysError & e) { if (unlink(tempLink.c_str()) == -1) printError("unable to unlink '%1%'", tempLink); if (errno == EMLINK) { @@ -240,7 +242,7 @@ void LocalStore::optimisePath_(Activity * act, OptimiseStats & stats, debug("'%s' has reached maximum number of links", linkPath); return; } - throw SysError("cannot rename '%1%' to '%2%'", tempLink, path); + throw; } stats.filesLinked++; diff --git a/src/libutil/filesystem.cc b/src/libutil/filesystem.cc index e67383e6f..33a8d81a6 100644 --- a/src/libutil/filesystem.cc +++ b/src/libutil/filesystem.cc @@ -34,11 +34,16 @@ void replaceSymlink(const Path & target, const Path & link, throw; } - if (rename(tmp.c_str(), link.c_str()) != 0) - throw SysError("renaming '%1%' to '%2%'", tmp, link); + moveFile(tmp, link); break; } } +void moveFile(const Path & oldName, const Path & newName) +{ + if (::rename(oldName.c_str(), newName.c_str())) + throw SysError("renaming '%1%' to '%2%'", oldName, newName); +} + } diff --git a/src/libutil/util.hh b/src/libutil/util.hh index 29227ecc6..564d36e79 100644 --- a/src/libutil/util.hh +++ b/src/libutil/util.hh @@ -168,6 +168,8 @@ void createSymlink(const Path & target, const Path & link, void replaceSymlink(const Path & target, const Path & link, std::optional mtime = {}); +void moveFile(const Path & src, const Path & dst); + /* Wrappers arount read()/write() that read/write exactly the requested number of bytes. */