Chromium Code Reviews
chromiumcodereview-hr@appspot.gserviceaccount.com (chromiumcodereview-hr) | Please choose your nickname with Settings | Help | Chromium Project | Gerrit Changes | Sign out
(59)

Unified Diff: content/browser/download/base_file_posix.cc

Issue 639253004: Allow POSIX callers to specify a new file's mode. (Closed) Base URL: https://chromium.googlesource.com/chromium/src.git@master
Patch Set: Use ScopedFD, rename function, adhere to the bool return value. Created 6 years, 2 months ago
Use n/p to move between diff chunks; N/P to move between comments. Draft comments are only viewable by you.
Jump to:
View side-by-side diff with in-line comments
Download patch
« base/files/file_util_posix.cc ('K') | « base/files/file_util_unittest.cc ('k') | no next file » | no next file with comments »
Expand Comments ('e') | Collapse Comments ('c') | Show Comments Hide Comments ('s')
Index: content/browser/download/base_file_posix.cc
diff --git a/content/browser/download/base_file_posix.cc b/content/browser/download/base_file_posix.cc
index b5d8e01442244b0f0a4fb45b60d3060618bb4786..ebdb92e3bead8d76eeaf047a91c09d0d88194f8a 100644
--- a/content/browser/download/base_file_posix.cc
+++ b/content/browser/download/base_file_posix.cc
@@ -11,31 +11,32 @@ namespace content {
DownloadInterruptReason BaseFile::MoveFileAndAdjustPermissions(
const base::FilePath& new_path) {
- // Similarly, on Unix, we're moving a temp file created with permissions 600
- // to |new_path|. Here, we try to fix up the destination file with appropriate
- // permissions.
- struct stat st;
- // First check the file existence and create an empty file if it doesn't
- // exist.
+ // Move a temporary file created with mode 0600 to |new_path|. If
+ // |new_path| does not already exist, set the correct mode on the new file
+ // (honoring the user's umask).
+ mode_t mask = umask(0);
+ (void) umask(mask);
+ mode_t mode = 0666 & ~mask;
+
if (!base::PathExists(new_path)) {
- int write_error = base::WriteFile(new_path, "", 0);
- if (write_error < 0)
+ if (!base::WriteFileWithMode(new_path, "", 0, mode))
return LogSystemError("WriteFile", errno);
}
- int stat_error = stat(new_path.value().c_str(), &st);
- bool stat_succeeded = (stat_error == 0);
- if (!stat_succeeded)
- LogSystemError("stat", errno);
-
- if (!base::Move(full_path_, new_path))
- return LogSystemError("Move", errno);
-
- if (stat_succeeded) {
- // On Windows file systems (FAT, NTFS), chmod fails. This is OK.
- int chmod_error = chmod(new_path.value().c_str(), st.st_mode);
- if (chmod_error < 0)
- LogSystemError("chmod", errno);
+
+ // If rename(2) fails, fall back to base::Move.
+ if (rename(full_path_.value().c_str(), new_path.value().c_str())) {
+ if (!base::Move(full_path_, new_path))
+ return LogSystemError("Move", errno);
}
+
+ // If |base::Move| had to copy the file (e.g. because the source is on a
+ // different volume than |new_path|, we must re-set the mode. This is
+ // racy but may be the best we can do.
+ //
+ // On Windows file systems (FAT, NTFS), chmod fails. This is OK.
+ if (chmod(new_path.value().c_str(), mode))
+ (void) LogSystemError("chmod", errno);
+
return DOWNLOAD_INTERRUPT_REASON_NONE;
}
« base/files/file_util_posix.cc ('K') | « base/files/file_util_unittest.cc ('k') | no next file » | no next file with comments »

Powered by Google App Engine
This is Rietveld 408576698