From 5411fbf20467c5dd561048311519056688b1154d Mon Sep 17 00:00:00 2001 From: Artemis Tosini Date: Sun, 19 May 2024 22:07:58 +0000 Subject: [PATCH] libutil: Create chmodPath function Move the identical static `chmod_` functions in libstore to libutil. the function is called `chmodPath` instead of `chmod` as otherwise it will shadow the standard library chmod in the nix namespace, which is somewhat confusing. Change-Id: I7b5ce379c6c602e3d3a1bbc49dbb70b1ae8f7bad --- src/libstore/build/derivation-goal.cc | 11 ++--------- src/libstore/build/local-derivation-goal.cc | 20 +++++++------------- src/libutil/util.cc | 8 ++++++-- src/libutil/util.hh | 7 +++++++ 4 files changed, 22 insertions(+), 24 deletions(-) diff --git a/src/libstore/build/derivation-goal.cc b/src/libstore/build/derivation-goal.cc index 3f24da276..5fa5deb7c 100644 --- a/src/libstore/build/derivation-goal.cc +++ b/src/libstore/build/derivation-goal.cc @@ -786,13 +786,6 @@ void DerivationGoal::tryLocalBuild() { } -static void chmod_(const Path & path, mode_t mode) -{ - if (chmod(path.c_str(), mode) == -1) - throw SysError("setting permissions on '%s'", path); -} - - /* Move/rename path 'src' to 'dst'. Temporarily make 'src' writable if it's a directory and we're not root (to be able to update the directory's parent link ".."). */ @@ -803,12 +796,12 @@ static void movePath(const Path & src, const Path & dst) bool changePerm = (geteuid() && S_ISDIR(st.st_mode) && !(st.st_mode & S_IWUSR)); if (changePerm) - chmod_(src, st.st_mode | S_IWUSR); + chmodPath(src, st.st_mode | S_IWUSR); renameFile(src, dst); if (changePerm) - chmod_(dst, st.st_mode); + chmodPath(dst, st.st_mode); } diff --git a/src/libstore/build/local-derivation-goal.cc b/src/libstore/build/local-derivation-goal.cc index cdbd0f5a7..5c36a3ac2 100644 --- a/src/libstore/build/local-derivation-goal.cc +++ b/src/libstore/build/local-derivation-goal.cc @@ -272,12 +272,6 @@ void LocalDerivationGoal::tryLocalBuild() started(); } -static void chmod_(const Path & path, mode_t mode) -{ - if (chmod(path.c_str(), mode) == -1) - throw SysError("setting permissions on '%s'", path); -} - /* Move/rename path 'src' to 'dst'. Temporarily make 'src' writable if it's a directory and we're not root (to be able to update the @@ -289,12 +283,12 @@ static void movePath(const Path & src, const Path & dst) bool changePerm = (geteuid() && S_ISDIR(st.st_mode) && !(st.st_mode & S_IWUSR)); if (changePerm) - chmod_(src, st.st_mode | S_IWUSR); + chmodPath(src, st.st_mode | S_IWUSR); renameFile(src, dst); if (changePerm) - chmod_(dst, st.st_mode); + chmodPath(dst, st.st_mode); } @@ -696,7 +690,7 @@ void LocalDerivationGoal::startBuilder() instead.) */ Path chrootTmpDir = chrootRootDir + "/tmp"; createDirs(chrootTmpDir); - chmod_(chrootTmpDir, 01777); + chmodPath(chrootTmpDir, 01777); /* Create a /etc/passwd with entries for the build user and the nobody account. The latter is kind of a hack to support @@ -721,7 +715,7 @@ void LocalDerivationGoal::startBuilder() build user. */ Path chrootStoreDir = chrootRootDir + worker.store.storeDir; createDirs(chrootStoreDir); - chmod_(chrootStoreDir, 01775); + chmodPath(chrootStoreDir, 01775); if (buildUser && chown(chrootStoreDir.c_str(), 0, buildUser->getGID()) == -1) throw SysError("cannot change ownership of '%1%'", chrootStoreDir); @@ -1862,7 +1856,7 @@ void LocalDerivationGoal::runChild() auto dst = chrootRootDir + i.first; createDirs(dirOf(dst)); writeFile(dst, std::string_view((const char *) sh, sizeof(sh))); - chmod_(dst, 0555); + chmodPath(dst, 0555); } else #endif doBind(i.second.source, chrootRootDir + i.first, i.second.optional); @@ -1900,7 +1894,7 @@ void LocalDerivationGoal::runChild() /* Make sure /dev/pts/ptmx is world-writable. With some Linux versions, it is created with permissions 0. */ - chmod_(chrootRootDir + "/dev/pts/ptmx", 0666); + chmodPath(chrootRootDir + "/dev/pts/ptmx", 0666); } else { if (errno != EINVAL) throw SysError("mounting /dev/pts"); @@ -1911,7 +1905,7 @@ void LocalDerivationGoal::runChild() /* Make /etc unwritable */ if (!parsedDrv->useUidRange()) - chmod_(chrootRootDir + "/etc", 0555); + chmodPath(chrootRootDir + "/etc", 0555); /* Unshare this mount namespace. This is necessary because pivot_root() below changes the root of the mount diff --git a/src/libutil/util.cc b/src/libutil/util.cc index bc2dd1802..2c0fcc897 100644 --- a/src/libutil/util.cc +++ b/src/libutil/util.cc @@ -184,6 +184,11 @@ Path canonPath(PathView path, bool resolveSymlinks) return s.empty() ? "/" : std::move(s); } +void chmodPath(const Path & path, mode_t mode) +{ + if (chmod(path.c_str(), mode) == -1) + throw SysError("setting permissions on '%s'", path); +} Path dirOf(const PathView path) { @@ -1799,8 +1804,7 @@ AutoCloseFD createUnixDomainSocket(const Path & path, mode_t mode) bind(fdSocket.get(), path); - if (chmod(path.c_str(), mode) == -1) - throw SysError("changing permissions on '%1%'", path); + chmodPath(path.c_str(), mode); if (listen(fdSocket.get(), 100) == -1) throw SysError("cannot listen on socket '%1%'", path); diff --git a/src/libutil/util.hh b/src/libutil/util.hh index 914d6cce0..14868776c 100644 --- a/src/libutil/util.hh +++ b/src/libutil/util.hh @@ -77,6 +77,13 @@ Path absPath(Path path, */ Path canonPath(PathView path, bool resolveSymlinks = false); +/** + * Change the permissions of a path + * Not called `chmod` as it shadows and could be confused with + * `int chmod(char *, mode_t)`, which does not handle errors + */ +void chmodPath(const Path & path, mode_t mode); + /** * @return The directory part of the given canonical path, i.e., * everything before the final `/`. If the path is the root or an