Skip to content

Commit

Permalink
Revert "Allow POSIX callers to specify a new file's mode."
Browse files Browse the repository at this point in the history
This reverts commit 3457f73.

Breaks FileUtilTest.WriteFileWithMode on Android.

BUG=409416,362887

Review URL: https://codereview.chromium.org/687163005

Cr-Commit-Position: refs/heads/master@{#301968}
  • Loading branch information
Mike Wittman committed Oct 29, 2014
1 parent 8ed14e8 commit e0c8785
Show file tree
Hide file tree
Showing 4 changed files with 21 additions and 67 deletions.
8 changes: 0 additions & 8 deletions base/files/file_util.h
Original file line number Diff line number Diff line change
Expand Up @@ -322,21 +322,13 @@ BASE_EXPORT int ReadFile(const FilePath& filename, char* data, int max_size);

// Writes the given buffer into the file, overwriting any data that was
// previously there. Returns the number of bytes written, or -1 on error.
// The file will be created with a default mode (POSIX) or security
// descriptor (Windows). To specify a mode on POSIX, use |WriteFileWithMode|.
BASE_EXPORT int WriteFile(const FilePath& filename, const char* data,
int size);

#if defined(OS_POSIX)
// Appends |data| to |fd|. Does not close |fd| when done. Returns true iff
// |size| bytes of |data| were written to |fd|.
BASE_EXPORT bool WriteFileDescriptor(const int fd, const char* data, int size);

// Writes the given buffer into the file, overwriting any data that was
// previously there. Returns true iff |size| bytes of |data| were written to
// |filename|. The file is created with the given |mode|.
BASE_EXPORT bool WriteFileWithMode(const FilePath& filename, const char* data,
int size, mode_t mode);
#endif

// Appends |data| to |filename|. Returns true iff |size| bytes of |data| were
Expand Down
9 changes: 0 additions & 9 deletions base/files/file_util_posix.cc
Original file line number Diff line number Diff line change
Expand Up @@ -695,15 +695,6 @@ int WriteFile(const FilePath& filename, const char* data, int size) {
return bytes_written;
}

bool WriteFileWithMode(const FilePath& filename, const char* data, int size,
mode_t mode) {
ThreadRestrictions::AssertIOAllowed();
ScopedFD file(HANDLE_EINTR(creat(filename.value().c_str(), mode)));
if (!file.is_valid())
return false;
return WriteFileDescriptor(file.get(), data, size);
}

bool WriteFileDescriptor(const int fd, const char* data, int size) {
// Allow for partial writes.
ssize_t bytes_written_total = 0;
Expand Down
22 changes: 0 additions & 22 deletions base/files/file_util_unittest.cc
Original file line number Diff line number Diff line change
Expand Up @@ -2143,28 +2143,6 @@ TEST_F(FileUtilTest, TouchFile) {
file_info.last_modified.ToInternalValue());
}

#if defined(OS_POSIX)
TEST_F(FileUtilTest, WriteFileWithMode) {
FilePath data_dir =
temp_dir_.path().Append(FILE_PATH_LITERAL("FilePathTest"));

// Create a fresh, empty copy of this directory.
if (PathExists(data_dir)) {
ASSERT_TRUE(DeleteFile(data_dir, true));
}
ASSERT_TRUE(CreateDirectory(data_dir));

FilePath foobar(data_dir.Append(FILE_PATH_LITERAL("foobar.txt")));
std::string data("hello");
mode_t mode = 0644;
ASSERT_TRUE(WriteFileWithMode(foobar, data.c_str(), data.length(), mode));

struct stat status;
ASSERT_EQ(0, stat(foobar.value().c_str(), &status));
ASSERT_EQ(mode, status.st_mode & 0777);
}
#endif

TEST_F(FileUtilTest, IsDirectoryEmpty) {
FilePath empty_dir = temp_dir_.path().Append(FILE_PATH_LITERAL("EmptyDir"));

Expand Down
49 changes: 21 additions & 28 deletions content/browser/download/base_file_posix.cc
Original file line number Diff line number Diff line change
Expand Up @@ -11,38 +11,31 @@ namespace content {

DownloadInterruptReason BaseFile::MoveFileAndAdjustPermissions(
const base::FilePath& new_path) {
// Move |full_path_|, created with mode 0600, to |new_path|.
//
// If |new_path| does not already exist, create it. The kernel will apply
// the user's umask to the mode 0666.
// 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.
if (!base::PathExists(new_path)) {
if (!base::WriteFileWithMode(new_path, "", 0, 0666))
int write_error = base::WriteFile(new_path, "", 0);
if (write_error < 0)
return LogSystemError("WriteFile", errno);
}

// Whether newly-created or pre-existing, get the mode of the file named
// |new_path|.
mode_t mode;
struct stat status;
if (stat(new_path.value().c_str(), &status)) {
return LogSystemError("WriteFile", errno);
}
mode = status.st_mode & 0777;

// 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);
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 |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;
}

Expand Down

0 comments on commit e0c8785

Please sign in to comment.