Skip to content

Commit 82f337d

Browse files
committed
{libutil,libstore}: Factor out chmodIfNeeded
Using std::filesystem::path directly because we need .c_str() anyway to interact with chmod. Path/string views don't have to be null-terminated.
1 parent c99edc8 commit 82f337d

File tree

4 files changed

+57
-4
lines changed

4 files changed

+57
-4
lines changed

src/libstore/local-store.cc

Lines changed: 1 addition & 4 deletions
Original file line numberDiff line numberDiff line change
@@ -136,13 +136,10 @@ LocalStore::LocalStore(
136136
for (auto & perUserDir : {profilesDir + "/per-user", gcRootsDir + "/per-user"}) {
137137
createDirs(perUserDir);
138138
if (!readOnly) {
139-
auto st = lstat(perUserDir);
140-
141139
// Skip chmod call if the directory already has the correct permissions (0755).
142140
// This is to avoid failing when the executing user lacks permissions to change the directory's permissions
143141
// even if it would be no-op.
144-
if ((st.st_mode & (S_IRWXU | S_IRWXG | S_IRWXO)) != 0755 && chmod(perUserDir.c_str(), 0755) == -1)
145-
throw SysError("could not set permissions on '%s' to 755", perUserDir);
142+
chmodIfNeeded(perUserDir, 0755, S_IRWXU | S_IRWXG | S_IRWXO);
146143
}
147144
}
148145

src/libutil-tests/file-system.cc

Lines changed: 26 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -275,4 +275,30 @@ TEST(makeParentCanonical, root)
275275
{
276276
ASSERT_EQ(makeParentCanonical("/"), "/");
277277
}
278+
279+
/* ----------------------------------------------------------------------------
280+
* chmodIfNeeded
281+
* --------------------------------------------------------------------------*/
282+
283+
TEST(chmodIfNeeded, works)
284+
{
285+
auto [autoClose_, tmpFile] = nix::createTempFile();
286+
auto deleteTmpFile = AutoDelete(tmpFile);
287+
288+
auto modes = std::vector<mode_t>{0755, 0644, 0422, 0600, 0777};
289+
for (mode_t oldMode : modes) {
290+
for (mode_t newMode : modes) {
291+
chmod(tmpFile.c_str(), oldMode);
292+
bool permissionsChanged = false;
293+
ASSERT_NO_THROW(permissionsChanged = chmodIfNeeded(tmpFile, newMode));
294+
ASSERT_EQ(permissionsChanged, oldMode != newMode);
295+
}
296+
}
297+
}
298+
299+
TEST(chmodIfNeeded, nonexistent)
300+
{
301+
ASSERT_THROW(chmodIfNeeded("/schnitzel/darmstadt/pommes", 0755), SysError);
302+
}
303+
278304
}

src/libutil/file-system.cc

Lines changed: 14 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -776,4 +776,18 @@ std::filesystem::path makeParentCanonical(const std::filesystem::path & rawPath)
776776
}
777777
}
778778

779+
bool chmodIfNeeded(const fs::path & path, mode_t mode, mode_t mask)
780+
{
781+
auto pathString = path.string();
782+
auto prevMode = lstat(pathString).st_mode;
783+
784+
if (((prevMode ^ mode) & mask) == 0)
785+
return false;
786+
787+
if (chmod(pathString.c_str(), mode) != 0)
788+
throw SysError("could not set permissions on '%s' to %o", pathString, mode);
789+
790+
return true;
791+
}
792+
779793
} // namespace nix

src/libutil/file-system.hh

Lines changed: 16 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -366,4 +366,20 @@ typedef std::function<bool(const Path & path)> PathFilter;
366366

367367
extern PathFilter defaultPathFilter;
368368

369+
/**
370+
* Change permissions of a file only if necessary.
371+
*
372+
* @details
373+
* Skip chmod call if the directory already has the requested permissions.
374+
* This is to avoid failing when the executing user lacks permissions to change the
375+
* directory's permissions even if it would be no-op.
376+
*
377+
* @param path Path to the file to change the permissions for.
378+
* @param mode New file mode.
379+
* @param mask Used for checking if the file already has requested permissions.
380+
*
381+
* @return true if permissions changed, false otherwise.
382+
*/
383+
bool chmodIfNeeded(const std::filesystem::path & path, mode_t mode, mode_t mask = S_IRWXU | S_IRWXG | S_IRWXO);
384+
369385
}

0 commit comments

Comments
 (0)