diff options
author | brettw@chromium.org <brettw@chromium.org@0039d316-1c4b-4281-b951-d872f2087c98> | 2014-03-14 19:36:50 +0000 |
---|---|---|
committer | brettw@chromium.org <brettw@chromium.org@0039d316-1c4b-4281-b951-d872f2087c98> | 2014-03-14 19:36:50 +0000 |
commit | c0e895e2961bd05570b0d7660016914601bf8c7d (patch) | |
tree | 3104b59d5f4d22ad60a35a3bcefdb7caff27d3c0 | |
parent | 5fcc99e7cf20aa9d0c44e2fdaef207d9e45f907a (diff) | |
download | chromium_src-c0e895e2961bd05570b0d7660016914601bf8c7d.zip chromium_src-c0e895e2961bd05570b0d7660016914601bf8c7d.tar.gz chromium_src-c0e895e2961bd05570b0d7660016914601bf8c7d.tar.bz2 |
Implement ScopedFD in terms of ScopedGeneric.
Move to a new file base/files/scoped_file.h. I will also add ScopedFILE to here (currently in file_util.h) later.
I think there is a crash in the old code in content/browser/zygote_host/zygote_host_impl_linux.cc that this patch should fix. The old ScopedFD took the address of something in a vector that is being modified.
I removed SafeScopedFD from content/common/sandbox_linux/sandbox_linux.cc since base's ScopedFD not CHECKs on close failure (this is a more recent addition).
BUG=
R=agl@chromium.org, viettrungluu@chromium.org
Committed: https://src.chromium.org/viewvc/chrome?view=rev&revision=257001
Review URL: https://codereview.chromium.org/191673003
git-svn-id: svn://svn.chromium.org/chrome/trunk/src@257179 0039d316-1c4b-4281-b951-d872f2087c98
42 files changed, 332 insertions, 286 deletions
diff --git a/base/BUILD.gn b/base/BUILD.gn index d676f05..d07e10b 100644 --- a/base/BUILD.gn +++ b/base/BUILD.gn @@ -180,6 +180,8 @@ component("base") { "files/memory_mapped_file.h", "files/memory_mapped_file_posix.cc", "files/memory_mapped_file_win.cc", + "files/scoped_file.cc", + "files/scoped_file.h", "files/scoped_temp_dir.cc", "files/scoped_temp_dir.h", "float_util.h", diff --git a/base/base.gypi b/base/base.gypi index 141e498..88b2422 100644 --- a/base/base.gypi +++ b/base/base.gypi @@ -219,6 +219,8 @@ 'files/memory_mapped_file.h', 'files/memory_mapped_file_posix.cc', 'files/memory_mapped_file_win.cc', + 'files/scoped_file.cc', + 'files/scoped_file.h', 'files/scoped_platform_file_closer.cc', 'files/scoped_platform_file_closer.h', 'files/scoped_temp_dir.cc', diff --git a/base/debug/proc_maps_linux.cc b/base/debug/proc_maps_linux.cc index b7a5862..a956961 100644 --- a/base/debug/proc_maps_linux.cc +++ b/base/debug/proc_maps_linux.cc @@ -11,6 +11,7 @@ #endif #include "base/file_util.h" +#include "base/files/scoped_file.h" #include "base/strings/string_split.h" #if defined(OS_ANDROID) @@ -45,12 +46,11 @@ bool ReadProcMaps(std::string* proc_maps) { // file for details. const long kReadSize = sysconf(_SC_PAGESIZE); - int fd = HANDLE_EINTR(open("/proc/self/maps", O_RDONLY)); - if (fd == -1) { + base::ScopedFD fd(HANDLE_EINTR(open("/proc/self/maps", O_RDONLY))); + if (!fd.is_valid()) { DPLOG(ERROR) << "Couldn't open /proc/self/maps"; return false; } - file_util::ScopedFD fd_closer(&fd); proc_maps->clear(); while (true) { @@ -60,7 +60,7 @@ bool ReadProcMaps(std::string* proc_maps) { proc_maps->resize(pos + kReadSize); void* buffer = &(*proc_maps)[pos]; - ssize_t bytes_read = HANDLE_EINTR(read(fd, buffer, kReadSize)); + ssize_t bytes_read = HANDLE_EINTR(read(fd.get(), buffer, kReadSize)); if (bytes_read < 0) { DPLOG(ERROR) << "Couldn't read /proc/self/maps"; proc_maps->clear(); diff --git a/base/file_util.h b/base/file_util.h index 431569a..b86d8cb 100644 --- a/base/file_util.h +++ b/base/file_util.h @@ -426,32 +426,6 @@ struct ScopedFILEClose { // Automatically closes |FILE*|s. typedef scoped_ptr<FILE, ScopedFILEClose> ScopedFILE; -#if defined(OS_POSIX) -// Functor for |ScopedFD| (below). -struct ScopedFDClose { - inline void operator()(int* x) const { - if (x && *x >= 0) { - // It's important to crash here. - // There are security implications to not closing a file descriptor - // properly. As file descriptors are "capabilities", keeping them open - // would make the current process keep access to a resource. Much of - // Chrome relies on being able to "drop" such access. - // It's especially problematic on Linux with the setuid sandbox, where - // a single open directory would bypass the entire security model. - PCHECK(0 == IGNORE_EINTR(close(*x))); - } - } -}; - -// Automatically closes FDs (note: doesn't store the FD). -// TODO(viettrungluu): This is a very odd API, since (unlike |FILE*|s, you'll -// need to store the FD separately and keep its memory alive). This should -// probably be called |ScopedFDCloser| or something like that. -typedef scoped_ptr<int, ScopedFDClose> ScopedFD; -// Let new users use ScopedFDCloser already, while ScopedFD is replaced. -typedef ScopedFD ScopedFDCloser; -#endif // OS_POSIX - } // namespace file_util // Internal -------------------------------------------------------------------- diff --git a/base/file_util_posix.cc b/base/file_util_posix.cc index 7e7dc05..fb4ebcf 100644 --- a/base/file_util_posix.cc +++ b/base/file_util_posix.cc @@ -33,6 +33,7 @@ #include "base/basictypes.h" #include "base/files/file_enumerator.h" #include "base/files/file_path.h" +#include "base/files/scoped_file.h" #include "base/logging.h" #include "base/memory/scoped_ptr.h" #include "base/memory/singleton.h" @@ -172,15 +173,15 @@ int CreateAndOpenFdForTemporaryFile(FilePath directory, FilePath* path) { bool DetermineDevShmExecutable() { bool result = false; FilePath path; - int fd = CreateAndOpenFdForTemporaryFile(FilePath("/dev/shm"), &path); - if (fd >= 0) { - file_util::ScopedFD shm_fd_closer(&fd); + + ScopedFD fd(CreateAndOpenFdForTemporaryFile(FilePath("/dev/shm"), &path)); + if (fd.is_valid()) { DeleteFile(path, false); long sysconf_result = sysconf(_SC_PAGESIZE); CHECK_GE(sysconf_result, 0); size_t pagesize = static_cast<size_t>(sysconf_result); CHECK_GE(sizeof(pagesize), sizeof(sysconf_result)); - void *mapping = mmap(NULL, pagesize, PROT_READ, MAP_SHARED, fd, 0); + void *mapping = mmap(NULL, pagesize, PROT_READ, MAP_SHARED, fd.get(), 0); if (mapping != MAP_FAILED) { if (mprotect(mapping, pagesize, PROT_READ | PROT_EXEC) == 0) result = true; @@ -660,11 +661,10 @@ bool GetFileInfo(const FilePath& file_path, File::Info* results) { stat_wrapper_t file_info; #if defined(OS_ANDROID) if (file_path.IsContentUri()) { - int fd = OpenContentUriForRead(file_path); - if (fd < 0) + ScopedFD fd(OpenContentUriForRead(file_path)); + if (!fd.is_valid()) return false; - file_util::ScopedFD scoped_fd(&fd); - if (CallFstat(fd, &file_info) != 0) + if (CallFstat(fd.get(), &file_info) != 0) return false; } else { #endif // defined(OS_ANDROID) diff --git a/base/file_util_unittest.cc b/base/file_util_unittest.cc index 96b58582..8ee55ff 100644 --- a/base/file_util_unittest.cc +++ b/base/file_util_unittest.cc @@ -26,6 +26,7 @@ #include "base/file_util.h" #include "base/files/file_enumerator.h" #include "base/files/file_path.h" +#include "base/files/scoped_file.h" #include "base/files/scoped_temp_dir.h" #include "base/path_service.h" #include "base/strings/utf_string_conversions.h" @@ -2473,9 +2474,9 @@ TEST(ScopedFD, ScopedFDDoesClose) { char c = 0; ASSERT_EQ(0, pipe(fds)); const int write_end = fds[1]; - file_util::ScopedFDCloser read_end_closer(fds); + base::ScopedFD read_end_closer(fds[0]); { - file_util::ScopedFDCloser write_end_closer(fds + 1); + base::ScopedFD write_end_closer(fds[1]); } // This is the only thread. This file descriptor should no longer be valid. int ret = close(write_end); @@ -2489,14 +2490,14 @@ TEST(ScopedFD, ScopedFDDoesClose) { #if defined(GTEST_HAS_DEATH_TEST) void CloseWithScopedFD(int fd) { - file_util::ScopedFDCloser fd_closer(&fd); + base::ScopedFD fd_closer(fd); } #endif TEST(ScopedFD, ScopedFDCrashesOnCloseFailure) { int fds[2]; ASSERT_EQ(0, pipe(fds)); - file_util::ScopedFDCloser read_end_closer(fds); + base::ScopedFD read_end_closer(fds[0]); EXPECT_EQ(0, IGNORE_EINTR(close(fds[1]))); #if defined(GTEST_HAS_DEATH_TEST) // This is the only thread. This file descriptor should no longer be valid. diff --git a/base/files/scoped_file.cc b/base/files/scoped_file.cc new file mode 100644 index 0000000..39f064d --- /dev/null +++ b/base/files/scoped_file.cc @@ -0,0 +1,35 @@ +// Copyright 2014 The Chromium Authors. All rights reserved. +// Use of this source code is governed by a BSD-style license that can be +// found in the LICENSE file. + +#include "base/files/scoped_file.h" + +#include "base/logging.h" + +#if defined(OS_POSIX) +#include <unistd.h> + +#include "base/posix/eintr_wrapper.h" +#endif + +namespace base { +namespace internal { + +#if defined(OS_POSIX) + +// static +void ScopedFDCloseTraits::Free(int fd) { + // It's important to crash here. + // There are security implications to not closing a file descriptor + // properly. As file descriptors are "capabilities", keeping them open + // would make the current process keep access to a resource. Much of + // Chrome relies on being able to "drop" such access. + // It's especially problematic on Linux with the setuid sandbox, where + // a single open directory would bypass the entire security model. + PCHECK(0 == IGNORE_EINTR(close(fd))); +} + +#endif // OS_POSIX + +} // namespace internal +} // namespace base diff --git a/base/files/scoped_file.h b/base/files/scoped_file.h new file mode 100644 index 0000000..f5d7ecde --- /dev/null +++ b/base/files/scoped_file.h @@ -0,0 +1,47 @@ +// Copyright 2014 The Chromium Authors. All rights reserved. +// Use of this source code is governed by a BSD-style license that can be +// found in the LICENSE file. + +#ifndef BASE_FILES_SCOPED_FILE_H_ +#define BASE_FILES_SCOPED_FILE_H_ + +#include <stdio.h> + +#include "base/base_export.h" +#include "base/logging.h" +#include "base/scoped_generic.h" +#include "build/build_config.h" + +namespace base { + +namespace internal { + +#if defined(OS_POSIX) +struct BASE_EXPORT ScopedFDCloseTraits { + static int InvalidValue() { + return -1; + } + static void Free(int fd); +}; +#endif + +} // namespace internal + +#if defined(OS_POSIX) +// A low-level Posix file descriptor closer class. Use this when writing +// platform-specific code, especially that does non-file-like things with the +// FD (like sockets). +// +// If you're writing low-level Windows code, see base/win/scoped_handle.h +// which provides some additional functionality. +// +// If you're writing cross-platform code that deals with actual files, you +// should generally use base::File instead which can be constructed with a +// handle, and in addition to handling ownership, has convenient cross-platform +// file manipulation functions on it. +typedef ScopedGeneric<int, internal::ScopedFDCloseTraits> ScopedFD; +#endif + +} // namespace base + +#endif // BASE_FILES_SCOPED_FILE_H_ diff --git a/base/memory/discardable_memory_allocator_android.cc b/base/memory/discardable_memory_allocator_android.cc index 97e9abd..1588317 100644 --- a/base/memory/discardable_memory_allocator_android.cc +++ b/base/memory/discardable_memory_allocator_android.cc @@ -16,6 +16,7 @@ #include "base/basictypes.h" #include "base/containers/hash_tables.h" #include "base/file_util.h" +#include "base/files/scoped_file.h" #include "base/logging.h" #include "base/memory/discardable_memory.h" #include "base/memory/scoped_vector.h" @@ -65,14 +66,13 @@ bool CreateAshmemRegion(const char* name, size_t size, int* out_fd, void** out_address) { - int fd = ashmem_create_region(name, size); - if (fd < 0) { + base::ScopedFD fd(ashmem_create_region(name, size)); + if (!fd.is_valid()) { DLOG(ERROR) << "ashmem_create_region() failed"; return false; } - file_util::ScopedFD fd_closer(&fd); - const int err = ashmem_set_prot_region(fd, PROT_READ | PROT_WRITE); + const int err = ashmem_set_prot_region(fd.get(), PROT_READ | PROT_WRITE); if (err < 0) { DLOG(ERROR) << "Error " << err << " when setting protection of ashmem"; return false; @@ -82,14 +82,13 @@ bool CreateAshmemRegion(const char* name, // Lock() and Unlock(), data could get lost if they are not written to the // underlying file when Unlock() gets called. void* const address = mmap( - NULL, size, PROT_READ | PROT_WRITE, MAP_SHARED, fd, 0); + NULL, size, PROT_READ | PROT_WRITE, MAP_SHARED, fd.get(), 0); if (address == MAP_FAILED) { DPLOG(ERROR) << "Failed to map memory."; return false; } - ignore_result(fd_closer.release()); - *out_fd = fd; + *out_fd = fd.release(); *out_address = address; return true; } diff --git a/base/memory/shared_memory.h b/base/memory/shared_memory.h index 50b61c4..8e32e72 100644 --- a/base/memory/shared_memory.h +++ b/base/memory/shared_memory.h @@ -22,6 +22,7 @@ #if defined(OS_POSIX) #include "base/file_descriptor_posix.h" #include "base/file_util.h" +#include "base/files/scoped_file.h" #endif namespace base { @@ -259,7 +260,7 @@ class BASE_EXPORT SharedMemory { private: #if defined(OS_POSIX) && !defined(OS_NACL) - bool PrepareMapFile(file_util::ScopedFILE fp, file_util::ScopedFD readonly); + bool PrepareMapFile(file_util::ScopedFILE fp, base::ScopedFD readonly); bool FilePathForMemoryName(const std::string& mem_name, FilePath* path); void LockOrUnlockCommon(int function); #endif diff --git a/base/memory/shared_memory_posix.cc b/base/memory/shared_memory_posix.cc index 8780c04..1a90847 100644 --- a/base/memory/shared_memory_posix.cc +++ b/base/memory/shared_memory_posix.cc @@ -30,7 +30,6 @@ #include "third_party/ashmem/ashmem.h" #endif -using file_util::ScopedFD; using file_util::ScopedFILE; namespace base { @@ -132,8 +131,7 @@ bool SharedMemory::Create(const SharedMemoryCreateOptions& options) { ScopedFILE fp; bool fix_size = true; - int readonly_fd_storage = -1; - ScopedFD readonly_fd(&readonly_fd_storage); + ScopedFD readonly_fd; FilePath path; if (options.name_deprecated == NULL || options.name_deprecated->empty()) { @@ -145,8 +143,8 @@ bool SharedMemory::Create(const SharedMemoryCreateOptions& options) { if (fp) { // Also open as readonly so that we can ShareReadOnlyToProcess. - *readonly_fd = HANDLE_EINTR(open(path.value().c_str(), O_RDONLY)); - if (*readonly_fd < 0) { + readonly_fd.reset(HANDLE_EINTR(open(path.value().c_str(), O_RDONLY))); + if (!readonly_fd.is_valid()) { DPLOG(ERROR) << "open(\"" << path.value() << "\", O_RDONLY) failed"; fp.reset(); } @@ -198,8 +196,8 @@ bool SharedMemory::Create(const SharedMemoryCreateOptions& options) { } // Also open as readonly so that we can ShareReadOnlyToProcess. - *readonly_fd = HANDLE_EINTR(open(path.value().c_str(), O_RDONLY)); - if (*readonly_fd < 0) { + readonly_fd.reset(HANDLE_EINTR(open(path.value().c_str(), O_RDONLY))); + if (!readonly_fd.is_valid()) { DPLOG(ERROR) << "open(\"" << path.value() << "\", O_RDONLY) failed"; close(fd); fd = -1; @@ -265,10 +263,8 @@ bool SharedMemory::Open(const std::string& name, bool read_only) { const char *mode = read_only ? "r" : "r+"; ScopedFILE fp(base::OpenFile(path, mode)); - int readonly_fd_storage = -1; - ScopedFD readonly_fd(&readonly_fd_storage); - *readonly_fd = HANDLE_EINTR(open(path.value().c_str(), O_RDONLY)); - if (*readonly_fd < 0) { + ScopedFD readonly_fd(HANDLE_EINTR(open(path.value().c_str(), O_RDONLY))); + if (!readonly_fd.is_valid()) { DPLOG(ERROR) << "open(\"" << path.value() << "\", O_RDONLY) failed"; } return PrepareMapFile(fp.Pass(), readonly_fd.Pass()); @@ -353,7 +349,7 @@ void SharedMemory::UnlockDeprecated() { bool SharedMemory::PrepareMapFile(ScopedFILE fp, ScopedFD readonly_fd) { DCHECK_EQ(-1, mapped_file_); DCHECK_EQ(-1, readonly_mapped_file_); - if (fp == NULL || *readonly_fd < 0) return false; + if (fp == NULL || !readonly_fd.is_valid()) return false; // This function theoretically can block on the disk, but realistically // the temporary files we create will just go into the buffer cache @@ -364,7 +360,7 @@ bool SharedMemory::PrepareMapFile(ScopedFILE fp, ScopedFD readonly_fd) { struct stat readonly_st = {}; if (fstat(fileno(fp.get()), &st)) NOTREACHED(); - if (fstat(*readonly_fd, &readonly_st)) + if (fstat(readonly_fd.get(), &readonly_st)) NOTREACHED(); if (st.st_dev != readonly_st.st_dev || st.st_ino != readonly_st.st_ino) { LOG(ERROR) << "writable and read-only inodes don't match; bailing"; @@ -381,7 +377,7 @@ bool SharedMemory::PrepareMapFile(ScopedFILE fp, ScopedFD readonly_fd) { } } inode_ = st.st_ino; - readonly_mapped_file_ = *readonly_fd.release(); + readonly_mapped_file_ = readonly_fd.release(); return true; } diff --git a/base/posix/unix_domain_socket_linux_unittest.cc b/base/posix/unix_domain_socket_linux_unittest.cc index 22bb172..bb7154f 100644 --- a/base/posix/unix_domain_socket_linux_unittest.cc +++ b/base/posix/unix_domain_socket_linux_unittest.cc @@ -9,6 +9,7 @@ #include "base/bind.h" #include "base/bind_helpers.h" #include "base/file_util.h" +#include "base/files/scoped_file.h" #include "base/pickle.h" #include "base/posix/unix_domain_socket_linux.h" #include "base/synchronization/waitable_event.h" @@ -25,8 +26,8 @@ TEST(UnixDomainSocketTest, SendRecvMsgAbortOnReplyFDClose) { int fds[2]; ASSERT_EQ(0, socketpair(AF_UNIX, SOCK_SEQPACKET, 0, fds)); - file_util::ScopedFD scoped_fd0(&fds[0]); - file_util::ScopedFD scoped_fd1(&fds[1]); + ScopedFD scoped_fd0(fds[0]); + ScopedFD scoped_fd1(fds[1]); // Have the thread send a synchronous message via the socket. Pickle request; @@ -62,7 +63,7 @@ TEST(UnixDomainSocketTest, SendRecvMsgAvoidsSIGPIPE) { ASSERT_EQ(0, sigaction(SIGPIPE, &act, &oldact)); int fds[2]; ASSERT_EQ(0, socketpair(AF_UNIX, SOCK_SEQPACKET, 0, fds)); - file_util::ScopedFD scoped_fd1(&fds[1]); + ScopedFD scoped_fd1(fds[1]); ASSERT_EQ(0, IGNORE_EINTR(close(fds[0]))); // Have the thread send a synchronous message via the socket. Unless the diff --git a/base/process/kill_mac.cc b/base/process/kill_mac.cc index 5ebca5d..1589a64 100644 --- a/base/process/kill_mac.cc +++ b/base/process/kill_mac.cc @@ -10,6 +10,7 @@ #include <sys/wait.h> #include "base/file_util.h" +#include "base/files/scoped_file.h" #include "base/logging.h" #include "base/posix/eintr_wrapper.h" @@ -76,15 +77,13 @@ void WaitForChildToDie(pid_t child, int timeout) { int result; - int kq = HANDLE_EINTR(kqueue()); - if (kq == -1) { + ScopedFD kq(HANDLE_EINTR(kqueue())); + if (!kq.is_valid()) { DPLOG(ERROR) << "kqueue()"; } else { - file_util::ScopedFD auto_close_kq(&kq); - struct kevent change = {0}; EV_SET(&change, child, EVFILT_PROC, EV_ADD, NOTE_EXIT, 0, NULL); - result = HANDLE_EINTR(kevent(kq, &change, 1, NULL, 0, NULL)); + result = HANDLE_EINTR(kevent(kq.get(), &change, 1, NULL, 0, NULL)); if (result == -1) { if (errno != ESRCH) { @@ -120,7 +119,7 @@ void WaitForChildToDie(pid_t child, int timeout) { struct kevent event = {0}; while (remaining_delta.InMilliseconds() > 0) { const struct timespec remaining_timespec = remaining_delta.ToTimeSpec(); - result = kevent(kq, NULL, 0, &event, 1, &remaining_timespec); + result = kevent(kq.get(), NULL, 0, &event, 1, &remaining_timespec); if (result == -1 && errno == EINTR) { remaining_delta = deadline - TimeTicks::Now(); result = 0; diff --git a/base/process/kill_posix.cc b/base/process/kill_posix.cc index 99d70d9..8187c38 100644 --- a/base/process/kill_posix.cc +++ b/base/process/kill_posix.cc @@ -10,6 +10,7 @@ #include <unistd.h> #include "base/file_util.h" +#include "base/files/scoped_file.h" #include "base/logging.h" #include "base/posix/eintr_wrapper.h" #include "base/process/process_iterator.h" @@ -273,16 +274,15 @@ static bool WaitForSingleNonChildProcess(ProcessHandle handle, DCHECK_GT(handle, 0); DCHECK(wait.InMilliseconds() == base::kNoTimeout || wait > base::TimeDelta()); - int kq = kqueue(); - if (kq == -1) { + ScopedFD kq(kqueue()); + if (!kq.is_valid()) { DPLOG(ERROR) << "kqueue"; return false; } - file_util::ScopedFD kq_closer(&kq); struct kevent change = {0}; EV_SET(&change, handle, EVFILT_PROC, EV_ADD, NOTE_EXIT, 0, NULL); - int result = HANDLE_EINTR(kevent(kq, &change, 1, NULL, 0, NULL)); + int result = HANDLE_EINTR(kevent(kq.get(), &change, 1, NULL, 0, NULL)); if (result == -1) { if (errno == ESRCH) { // If the process wasn't found, it must be dead. @@ -316,7 +316,7 @@ static bool WaitForSingleNonChildProcess(ProcessHandle handle, remaining_timespec_ptr = &remaining_timespec; } - result = kevent(kq, NULL, 0, &event, 1, remaining_timespec_ptr); + result = kevent(kq.get(), NULL, 0, &event, 1, remaining_timespec_ptr); if (result == -1 && errno == EINTR) { if (!wait_forever) { diff --git a/base/process/launch_posix.cc b/base/process/launch_posix.cc index 84a05d6..79e74d5 100644 --- a/base/process/launch_posix.cc +++ b/base/process/launch_posix.cc @@ -26,6 +26,7 @@ #include "base/debug/stack_trace.h" #include "base/file_util.h" #include "base/files/dir_reader_posix.h" +#include "base/files/scoped_file.h" #include "base/logging.h" #include "base/memory/scoped_ptr.h" #include "base/posix/eintr_wrapper.h" @@ -332,14 +333,13 @@ bool LaunchProcess(const std::vector<std::string>& argv, // If a child process uses the readline library, the process block forever. // In BSD like OSes including OS X it is safe to assign /dev/null as stdin. // See http://crbug.com/56596. - int null_fd = HANDLE_EINTR(open("/dev/null", O_RDONLY)); - if (null_fd < 0) { + base::ScopedFD null_fd(HANDLE_EINTR(open("/dev/null", O_RDONLY))); + if (!null_fd.is_valid()) { RAW_LOG(ERROR, "Failed to open /dev/null"); _exit(127); } - file_util::ScopedFD null_fd_closer(&null_fd); - int new_fd = HANDLE_EINTR(dup2(null_fd, STDIN_FILENO)); + int new_fd = HANDLE_EINTR(dup2(null_fd.get(), STDIN_FILENO)); if (new_fd != STDIN_FILENO) { RAW_LOG(ERROR, "Failed to dup /dev/null for stdin"); _exit(127); diff --git a/base/test/launcher/test_launcher.cc b/base/test/launcher/test_launcher.cc index c25b00a..889244d 100644 --- a/base/test/launcher/test_launcher.cc +++ b/base/test/launcher/test_launcher.cc @@ -14,6 +14,7 @@ #include "base/environment.h" #include "base/file_util.h" #include "base/files/file_path.h" +#include "base/files/scoped_file.h" #include "base/format_macros.h" #include "base/lazy_instance.h" #include "base/logging.h" @@ -244,16 +245,14 @@ void DoLaunchChildTestProcess( options.new_process_group = true; base::FileHandleMappingVector fds_mapping; - file_util::ScopedFD output_file_fd_closer; + base::ScopedFD output_file_fd; if (redirect_stdio) { - int output_file_fd = open(output_file.value().c_str(), O_RDWR); - CHECK_GE(output_file_fd, 0); + output_file_fd.reset(open(output_file.value().c_str(), O_RDWR)); + CHECK(output_file_fd.is_valid()); - output_file_fd_closer.reset(&output_file_fd); - - fds_mapping.push_back(std::make_pair(output_file_fd, STDOUT_FILENO)); - fds_mapping.push_back(std::make_pair(output_file_fd, STDERR_FILENO)); + fds_mapping.push_back(std::make_pair(output_file_fd.get(), STDOUT_FILENO)); + fds_mapping.push_back(std::make_pair(output_file_fd.get(), STDERR_FILENO)); options.fds_to_remap = &fds_mapping; } #endif @@ -264,10 +263,10 @@ void DoLaunchChildTestProcess( if (redirect_stdio) { #if defined(OS_WIN) - FlushFileBuffers(handle.Get()); - handle.Close(); + FlushFileBuffers(handle.Get()); + handle.Close(); #elif defined(OS_POSIX) - output_file_fd_closer.reset(); + output_file_fd.reset(); #endif } diff --git a/chrome/browser/chromeos/system/automatic_reboot_manager.cc b/chrome/browser/chromeos/system/automatic_reboot_manager.cc index 1e00316..2ca1409 100644 --- a/chrome/browser/chromeos/system/automatic_reboot_manager.cc +++ b/chrome/browser/chromeos/system/automatic_reboot_manager.cc @@ -18,6 +18,7 @@ #include "base/callback.h" #include "base/file_util.h" #include "base/files/file_path.h" +#include "base/files/scoped_file.h" #include "base/location.h" #include "base/logging.h" #include "base/memory/ref_counted.h" @@ -56,15 +57,15 @@ const int kOneKilobyte = 1 << 10; // 1 kB in bytes. base::TimeDelta ReadTimeDeltaFromFile(const base::FilePath& path) { base::ThreadRestrictions::AssertIOAllowed(); - int fd = HANDLE_EINTR(open(path.value().c_str(), O_RDONLY | O_NOFOLLOW)); - if (fd < 0) + base::ScopedFD fd( + HANDLE_EINTR(open(path.value().c_str(), O_RDONLY | O_NOFOLLOW))); + if (!fd.is_valid()) return base::TimeDelta(); - file_util::ScopedFD fd_closer(&fd); std::string contents; char buffer[kOneKilobyte]; ssize_t length; - while ((length = read(fd, buffer, sizeof(buffer))) > 0) + while ((length = read(fd.get(), buffer, sizeof(buffer))) > 0) contents.append(buffer, length); double seconds; @@ -108,16 +109,16 @@ void SaveUpdateRebootNeededUptime() { if (uptime == kZeroTimeDelta) return; - int fd = HANDLE_EINTR(open(update_reboot_needed_uptime_file.value().c_str(), - O_CREAT | O_WRONLY | O_TRUNC | O_NOFOLLOW, - 0666)); - if (fd < 0) + base::ScopedFD fd(HANDLE_EINTR( + open(update_reboot_needed_uptime_file.value().c_str(), + O_CREAT | O_WRONLY | O_TRUNC | O_NOFOLLOW, + 0666))); + if (!fd.is_valid()) return; - file_util::ScopedFD fd_closer(&fd); std::string update_reboot_needed_uptime = base::DoubleToString(uptime.InSecondsF()); - base::WriteFileDescriptor(fd, update_reboot_needed_uptime.c_str(), + base::WriteFileDescriptor(fd.get(), update_reboot_needed_uptime.c_str(), update_reboot_needed_uptime.size()); } diff --git a/chrome/browser/extensions/api/messaging/native_message_process_host_unittest.cc b/chrome/browser/extensions/api/messaging/native_message_process_host_unittest.cc index a57fb74..cf5b80b 100644 --- a/chrome/browser/extensions/api/messaging/native_message_process_host_unittest.cc +++ b/chrome/browser/extensions/api/messaging/native_message_process_host_unittest.cc @@ -5,6 +5,7 @@ #include "base/bind.h" #include "base/file_util.h" #include "base/files/file_path.h" +#include "base/files/scoped_file.h" #include "base/files/scoped_temp_dir.h" #include "base/json/json_reader.h" #include "base/memory/scoped_ptr.h" @@ -214,8 +215,8 @@ TEST_F(NativeMessagingTest, SingleSendMessageWrite) { #else // defined(OS_WIN) base::PlatformFile pipe_handles[2]; ASSERT_EQ(0, pipe(pipe_handles)); - file_util::ScopedFD read_fd(pipe_handles); - file_util::ScopedFD write_fd(pipe_handles + 1); + base::ScopedFD read_fd(pipe_handles[0]); + base::ScopedFD write_fd(pipe_handles[1]); read_file = pipe_handles[0]; #endif // !defined(OS_WIN) diff --git a/chrome/browser/extensions/api/messaging/native_process_launcher_posix.cc b/chrome/browser/extensions/api/messaging/native_process_launcher_posix.cc index f4523fc..70353d3 100644 --- a/chrome/browser/extensions/api/messaging/native_process_launcher_posix.cc +++ b/chrome/browser/extensions/api/messaging/native_process_launcher_posix.cc @@ -6,6 +6,7 @@ #include "base/command_line.h" #include "base/file_util.h" +#include "base/files/scoped_file.h" #include "base/logging.h" #include "base/path_service.h" #include "base/posix/eintr_wrapper.h" @@ -58,18 +59,18 @@ bool NativeProcessLauncher::LaunchNativeProcess( LOG(ERROR) << "Bad read pipe"; return false; } - file_util::ScopedFD read_pipe_read_fd(&read_pipe_fds[0]); - file_util::ScopedFD read_pipe_write_fd(&read_pipe_fds[1]); - fd_map.push_back(std::make_pair(*read_pipe_write_fd, STDOUT_FILENO)); + base::ScopedFD read_pipe_read_fd(read_pipe_fds[0]); + base::ScopedFD read_pipe_write_fd(read_pipe_fds[1]); + fd_map.push_back(std::make_pair(read_pipe_write_fd.get(), STDOUT_FILENO)); int write_pipe_fds[2] = {0}; if (HANDLE_EINTR(pipe(write_pipe_fds)) != 0) { LOG(ERROR) << "Bad write pipe"; return false; } - file_util::ScopedFD write_pipe_read_fd(&write_pipe_fds[0]); - file_util::ScopedFD write_pipe_write_fd(&write_pipe_fds[1]); - fd_map.push_back(std::make_pair(*write_pipe_read_fd, STDIN_FILENO)); + base::ScopedFD write_pipe_read_fd(write_pipe_fds[0]); + base::ScopedFD write_pipe_write_fd(write_pipe_fds[1]); + fd_map.push_back(std::make_pair(write_pipe_read_fd.get(), STDIN_FILENO)); base::LaunchOptions options; options.fds_to_remap = &fd_map; @@ -82,8 +83,8 @@ bool NativeProcessLauncher::LaunchNativeProcess( write_pipe_read_fd.reset(); read_pipe_write_fd.reset(); - *read_file = *read_pipe_read_fd.release(); - *write_file = *write_pipe_write_fd.release(); + *read_file = read_pipe_read_fd.release(); + *write_file = write_pipe_write_fd.release(); return true; } diff --git a/chrome/browser/mac/relauncher.cc b/chrome/browser/mac/relauncher.cc index 5d8357f..e659793 100644 --- a/chrome/browser/mac/relauncher.cc +++ b/chrome/browser/mac/relauncher.cc @@ -19,6 +19,7 @@ #include "base/basictypes.h" #include "base/file_util.h" +#include "base/files/scoped_file.h" #include "base/logging.h" #include "base/mac/mac_logging.h" #include "base/mac/mac_util.h" @@ -132,8 +133,8 @@ bool RelaunchAppWithHelper(const std::string& helper, // write side of the pipe. In that process, the read side will be closed by // base::LaunchApp because it won't be present in fd_map, and the write side // will be remapped to kRelauncherSyncFD by fd_map. - file_util::ScopedFD pipe_read_fd(&pipe_fds[0]); - file_util::ScopedFD pipe_write_fd(&pipe_fds[1]); + base::ScopedFD pipe_read_fd(pipe_fds[0]); + base::ScopedFD pipe_write_fd(pipe_fds[1]); // Make sure kRelauncherSyncFD is a safe value. base::LaunchProcess will // preserve these three FDs in forked processes, so kRelauncherSyncFD should @@ -144,7 +145,7 @@ bool RelaunchAppWithHelper(const std::string& helper, kRelauncherSyncFD_must_not_conflict_with_stdio_fds); base::FileHandleMappingVector fd_map; - fd_map.push_back(std::make_pair(*pipe_write_fd, kRelauncherSyncFD)); + fd_map.push_back(std::make_pair(pipe_write_fd.get(), kRelauncherSyncFD)); base::LaunchOptions options; options.fds_to_remap = &fd_map; @@ -160,7 +161,7 @@ bool RelaunchAppWithHelper(const std::string& helper, // Synchronize with the relauncher process. char read_char; - int read_result = HANDLE_EINTR(read(*pipe_read_fd, &read_char, 1)); + int read_result = HANDLE_EINTR(read(pipe_read_fd.get(), &read_char, 1)); if (read_result != 1) { if (read_result < 0) { PLOG(ERROR) << "read"; @@ -185,9 +186,7 @@ namespace { // situations, it can be assumed that something went wrong with the parent // process and the best recovery approach is to attempt relaunch anyway. void RelauncherSynchronizeWithParent() { - // file_util::ScopedFD needs something non-const to operate on. - int relauncher_sync_fd = kRelauncherSyncFD; - file_util::ScopedFD relauncher_sync_fd_closer(&relauncher_sync_fd); + base::ScopedFD relauncher_sync_fd(kRelauncherSyncFD); int parent_pid = getppid(); @@ -201,22 +200,21 @@ void RelauncherSynchronizeWithParent() { } // Set up a kqueue to monitor the parent process for exit. - int kq = kqueue(); - if (kq < 0) { + base::ScopedFD kq(kqueue()); + if (!kq.is_valid()) { PLOG(ERROR) << "kqueue"; return; } - file_util::ScopedFD kq_closer(&kq); struct kevent change = { 0 }; EV_SET(&change, parent_pid, EVFILT_PROC, EV_ADD, NOTE_EXIT, 0, NULL); - if (kevent(kq, &change, 1, NULL, 0, NULL) == -1) { + if (kevent(kq.get(), &change, 1, NULL, 0, NULL) == -1) { PLOG(ERROR) << "kevent (add)"; return; } // Write a '\0' character to the pipe. - if (HANDLE_EINTR(write(relauncher_sync_fd, "", 1)) != 1) { + if (HANDLE_EINTR(write(relauncher_sync_fd.get(), "", 1)) != 1) { PLOG(ERROR) << "write"; return; } @@ -225,7 +223,7 @@ void RelauncherSynchronizeWithParent() { // write above to complete. The parent process is now free to exit. Wait for // that to happen. struct kevent event; - int events = kevent(kq, NULL, 0, &event, 1, NULL); + int events = kevent(kq.get(), NULL, 0, &event, 1, NULL); if (events != 1) { if (events < 0) { PLOG(ERROR) << "kevent (monitor)"; diff --git a/chrome/browser/process_singleton_mac_unittest.cc b/chrome/browser/process_singleton_mac_unittest.cc index 4246c27..b6a5b7b 100644 --- a/chrome/browser/process_singleton_mac_unittest.cc +++ b/chrome/browser/process_singleton_mac_unittest.cc @@ -9,6 +9,7 @@ #include "chrome/browser/process_singleton.h" #include "base/file_util.h" +#include "base/files/scoped_file.h" #include "base/path_service.h" #include "base/posix/eintr_wrapper.h" #include "chrome/common/chrome_constants.h" @@ -39,15 +40,13 @@ class ProcessSingletonMacTest : public PlatformTest { // Return |true| if the file exists and is locked. Forces a failure // in the containing test in case of error condition. bool IsLocked() { - int fd = HANDLE_EINTR(open(lock_path_.value().c_str(), O_RDONLY)); - if (fd == -1) { + base::ScopedFD fd(HANDLE_EINTR(open(lock_path_.value().c_str(), O_RDONLY))); + if (!fd.is_valid()) { EXPECT_EQ(ENOENT, errno) << "Unexpected error opening lockfile."; return false; } - file_util::ScopedFD auto_close(&fd); - - int rc = HANDLE_EINTR(flock(fd, LOCK_EX|LOCK_NB)); + int rc = HANDLE_EINTR(flock(fd.get(), LOCK_EX|LOCK_NB)); // Got the lock, so it wasn't already locked. Close releases. if (rc != -1) diff --git a/chrome/test/automation/proxy_launcher.cc b/chrome/test/automation/proxy_launcher.cc index 6171f28..e49c4d6 100644 --- a/chrome/test/automation/proxy_launcher.cc +++ b/chrome/test/automation/proxy_launcher.cc @@ -9,6 +9,7 @@ #include "base/environment.h" #include "base/file_util.h" #include "base/files/file_enumerator.h" +#include "base/files/scoped_file.h" #include "base/process/kill.h" #include "base/process/launch.h" #include "base/strings/string_number_conversions.h" @@ -489,12 +490,11 @@ bool ProxyLauncher::LaunchBrowserHelper(const LaunchState& state, #if defined(OS_WIN) options.start_hidden = !state.show_window; #elif defined(OS_POSIX) - int ipcfd = -1; - file_util::ScopedFD ipcfd_closer(&ipcfd); + base::ScopedFD ipcfd; base::FileHandleMappingVector fds; if (main_launch && automation_proxy_.get()) { - ipcfd = automation_proxy_->channel()->TakeClientFileDescriptor(); - fds.push_back(std::make_pair(ipcfd, + ipcfd.reset(automation_proxy_->channel()->TakeClientFileDescriptor()); + fds.push_back(std::make_pair(ipcfd.get(), kPrimaryIPCChannel + base::GlobalDescriptors::kBaseDescriptor)); options.fds_to_remap = &fds; } diff --git a/chrome/test/chromedriver/chrome_launcher.cc b/chrome/test/chromedriver/chrome_launcher.cc index 44a4ad8..1ca47c3 100644 --- a/chrome/test/chromedriver/chrome_launcher.cc +++ b/chrome/test/chromedriver/chrome_launcher.cc @@ -12,6 +12,7 @@ #include "base/command_line.h" #include "base/file_util.h" #include "base/files/file_path.h" +#include "base/files/scoped_file.h" #include "base/format_macros.h" #include "base/json/json_reader.h" #include "base/json/json_writer.h" @@ -250,15 +251,14 @@ Status LaunchDesktopChrome( #if defined(OS_POSIX) base::FileHandleMappingVector no_stderr; - int devnull = -1; - file_util::ScopedFD scoped_devnull(&devnull); + base::ScopedFD devnull; if (!CommandLine::ForCurrentProcess()->HasSwitch("verbose")) { // Redirect stderr to /dev/null, so that Chrome log spew doesn't confuse // users. - devnull = open("/dev/null", O_WRONLY); - if (devnull == -1) + devnull.reset(open("/dev/null", O_WRONLY)); + if (!devnull.is_valid()) return Status(kUnknownError, "couldn't open /dev/null"); - no_stderr.push_back(std::make_pair(devnull, STDERR_FILENO)); + no_stderr.push_back(std::make_pair(devnull.get(), STDERR_FILENO)); options.fds_to_remap = &no_stderr; } #endif diff --git a/chrome/utility/importer/firefox_importer_unittest_utils_mac.cc b/chrome/utility/importer/firefox_importer_unittest_utils_mac.cc index 6b21149..93961c5 100644 --- a/chrome/utility/importer/firefox_importer_unittest_utils_mac.cc +++ b/chrome/utility/importer/firefox_importer_unittest_utils_mac.cc @@ -9,6 +9,7 @@ #include "base/command_line.h" #include "base/file_util.h" #include "base/files/file_path.h" +#include "base/files/scoped_file.h" #include "base/message_loop/message_loop.h" #include "base/posix/global_descriptors.h" #include "base/process/kill.h" @@ -47,13 +48,12 @@ bool LaunchNSSDecrypterChildProcess(const base::FilePath& nss_path, base::LaunchOptions options; options.environ["DYLD_FALLBACK_LIBRARY_PATH"] = nss_path.value(); - int ipcfd = channel->TakeClientFileDescriptor(); - if (ipcfd == -1) + base::ScopedFD ipcfd(channel->TakeClientFileDescriptor()); + if (!ipcfd.is_valid()) return false; - file_util::ScopedFD client_file_descriptor_closer(&ipcfd); base::FileHandleMappingVector fds_to_map; - fds_to_map.push_back(std::pair<int,int>(ipcfd, + fds_to_map.push_back(std::pair<int,int>(ipcfd.get(), kPrimaryIPCChannel + base::GlobalDescriptors::kBaseDescriptor)); bool debug_on_start = CommandLine::ForCurrentProcess()->HasSwitch( diff --git a/components/nacl.gyp b/components/nacl.gyp index 990221e..d34c1ba 100644 --- a/components/nacl.gyp +++ b/components/nacl.gyp @@ -211,6 +211,7 @@ 'nacl/loader/nonsfi/irt_util.h', 'nacl/loader/nonsfi/nonsfi_main.cc', 'nacl/loader/nonsfi/nonsfi_main.h', + '../base/files/scoped_file.cc', '../base/posix/unix_domain_socket_linux.cc', '../content/common/child_process_sandbox_support_impl_shm_linux.cc', '../content/common/sandbox_linux/sandbox_bpf_base_policy_linux.cc', diff --git a/content/browser/child_process_launcher.cc b/content/browser/child_process_launcher.cc index cacf138..5d7b612 100644 --- a/content/browser/child_process_launcher.cc +++ b/content/browser/child_process_launcher.cc @@ -9,6 +9,7 @@ #include "base/bind.h" #include "base/command_line.h" #include "base/file_util.h" +#include "base/files/scoped_file.h" #include "base/logging.h" #include "base/memory/scoped_ptr.h" #include "base/metrics/histogram.h" @@ -228,7 +229,7 @@ class ChildProcessLauncher::Context base::ProcessHandle handle = base::kNullProcessHandle; // We need to close the client end of the IPC channel to reliably detect // child termination. - file_util::ScopedFD ipcfd_closer(&ipcfd); + base::ScopedFD ipcfd_closer(ipcfd); #if !defined(OS_MACOSX) GetContentClient()->browser()-> @@ -319,7 +320,7 @@ class ChildProcessLauncher::Context base::ProcessHandle handle) { #if defined(OS_ANDROID) // Finally close the ipcfd - file_util::ScopedFD ipcfd_closer(&ipcfd_); + base::ScopedFD ipcfd_closer(ipcfd_); #endif starting_ = false; process_.set_handle(handle); diff --git a/content/browser/zygote_host/zygote_host_impl_linux.cc b/content/browser/zygote_host/zygote_host_impl_linux.cc index 18e9b69..aaf4d3b 100644 --- a/content/browser/zygote_host/zygote_host_impl_linux.cc +++ b/content/browser/zygote_host/zygote_host_impl_linux.cc @@ -14,6 +14,7 @@ #include "base/environment.h" #include "base/file_util.h" #include "base/files/file_enumerator.h" +#include "base/files/scoped_file.h" #include "base/linux_util.h" #include "base/logging.h" #include "base/memory/linked_ptr.h" @@ -318,7 +319,7 @@ pid_t ZygoteHostImpl::ForkRequest( std::vector<int> fds; // Scoped pointers cannot be stored in containers, so we have to use a // linked_ptr. - std::vector<linked_ptr<file_util::ScopedFD> > autodelete_fds; + std::vector<linked_ptr<base::ScopedFD> > autodelete_fds; for (std::vector<FileDescriptorInfo>::const_iterator i = mapping.begin(); i != mapping.end(); ++i) { pickle.WriteUInt32(i->id); @@ -326,8 +327,7 @@ pid_t ZygoteHostImpl::ForkRequest( if (i->fd.auto_close) { // Auto-close means we need to close the FDs after they have been passed // to the other process. - linked_ptr<file_util::ScopedFD> ptr( - new file_util::ScopedFD(&(fds.back()))); + linked_ptr<base::ScopedFD> ptr(new base::ScopedFD(fds.back())); autodelete_fds.push_back(ptr); } } diff --git a/content/child/npapi/np_channel_base.cc b/content/child/npapi/np_channel_base.cc index 85ade5a..76845bd 100644 --- a/content/child/npapi/np_channel_base.cc +++ b/content/child/npapi/np_channel_base.cc @@ -6,6 +6,7 @@ #include "base/auto_reset.h" #include "base/containers/hash_tables.h" +#include "base/files/scoped_file.h" #include "base/lazy_instance.h" #include "base/strings/string_number_conversions.h" #include "base/threading/thread_local.h" @@ -68,8 +69,8 @@ NPChannelBase* NPChannelBase::GetChannel( // On POSIX the channel_handle conveys an FD (socket) which is duped by the // kernel during the IPC message exchange (via the SCM_RIGHTS mechanism). // Ensure we do not leak this FD. - int fd = channel_handle.socket.auto_close ? channel_handle.socket.fd : -1; - file_util::ScopedFD auto_close_fd(&fd); + base::ScopedFD fd(channel_handle.socket.auto_close ? + channel_handle.socket.fd : -1); #endif scoped_refptr<NPChannelBase> channel; @@ -86,7 +87,7 @@ NPChannelBase* NPChannelBase::GetChannel( if (!channel->channel_valid()) { channel->channel_handle_ = channel_handle; #if defined(OS_POSIX) - ignore_result(auto_close_fd.release()); + ignore_result(fd.release()); #endif if (mode & IPC::Channel::MODE_SERVER_FLAG) { channel->channel_handle_.name = diff --git a/content/common/sandbox_linux/sandbox_linux.cc b/content/common/sandbox_linux/sandbox_linux.cc index 5fd89f7..6f60584 100644 --- a/content/common/sandbox_linux/sandbox_linux.cc +++ b/content/common/sandbox_linux/sandbox_linux.cc @@ -15,6 +15,7 @@ #include "base/bind.h" #include "base/callback_helpers.h" #include "base/command_line.h" +#include "base/files/scoped_file.h" #include "base/logging.h" #include "base/memory/scoped_ptr.h" #include "base/memory/singleton.h" @@ -43,10 +44,6 @@ struct FDCloser { } }; -// Don't use base::ScopedFD since it doesn't CHECK that the file descriptor was -// closed. -typedef scoped_ptr<int, FDCloser> SafeScopedFD; - void LogSandboxStarted(const std::string& sandbox_name) { const CommandLine& command_line = *CommandLine::ForCurrentProcess(); const std::string process_type = @@ -203,25 +200,25 @@ int LinuxSandbox::GetStatus() { // of using the pid. bool LinuxSandbox::IsSingleThreaded() const { bool is_single_threaded = false; - int proc_self_task = OpenProcTaskFd(proc_fd_); + base::ScopedFD proc_self_task(OpenProcTaskFd(proc_fd_)); // In Debug mode, it's mandatory to be able to count threads to catch bugs. #if !defined(NDEBUG) // Using CHECK here since we want to check all the cases where // !defined(NDEBUG) // gets built. - CHECK_LE(0, proc_self_task) << "Could not count threads, the sandbox was not " - << "pre-initialized properly."; + CHECK(proc_self_task.is_valid()) + << "Could not count threads, the sandbox was not " + << "pre-initialized properly."; #endif // !defined(NDEBUG) - if (proc_self_task < 0) { + if (!proc_self_task.is_valid()) { // Pretend to be monothreaded if it can't be determined (for instance the // setuid sandbox is already engaged but no proc_fd_ is available). is_single_threaded = true; } else { - SafeScopedFD task_closer(&proc_self_task); is_single_threaded = - sandbox::ThreadHelpers::IsSingleThreaded(proc_self_task); + sandbox::ThreadHelpers::IsSingleThreaded(proc_self_task.get()); } return is_single_threaded; @@ -391,11 +388,10 @@ void LinuxSandbox::CheckForBrokenPromises(const std::string& process_type) { void LinuxSandbox::StopThreadAndEnsureNotCounted(base::Thread* thread) const { DCHECK(thread); - int proc_self_task = OpenProcTaskFd(proc_fd_); - PCHECK(proc_self_task >= 0); - SafeScopedFD task_closer(&proc_self_task); - CHECK( - sandbox::ThreadHelpers::StopThreadAndWatchProcFS(proc_self_task, thread)); + base::ScopedFD proc_self_task(OpenProcTaskFd(proc_fd_)); + PCHECK(proc_self_task.is_valid()); + CHECK(sandbox::ThreadHelpers::StopThreadAndWatchProcFS(proc_self_task.get(), + thread)); } } // namespace content diff --git a/content/common/sandbox_mac.mm b/content/common/sandbox_mac.mm index 1ec3795..94c3c3d 100644 --- a/content/common/sandbox_mac.mm +++ b/content/common/sandbox_mac.mm @@ -17,6 +17,7 @@ extern "C" { #include "base/command_line.h" #include "base/compiler_specific.h" #include "base/file_util.h" +#include "base/files/scoped_file.h" #include "base/mac/bundle_locations.h" #include "base/mac/mac_util.h" #include "base/mac/scoped_cftyperef.h" @@ -607,16 +608,15 @@ bool Sandbox::SandboxIsCurrentlyActive() { // static base::FilePath Sandbox::GetCanonicalSandboxPath(const base::FilePath& path) { - int fd = HANDLE_EINTR(open(path.value().c_str(), O_RDONLY)); - if (fd < 0) { + base::ScopedFD fd(HANDLE_EINTR(open(path.value().c_str(), O_RDONLY))); + if (!fd.is_valid()) { DPLOG(FATAL) << "GetCanonicalSandboxPath() failed for: " << path.value(); return path; } - file_util::ScopedFD file_closer(&fd); base::FilePath::CharType canonical_path[MAXPATHLEN]; - if (HANDLE_EINTR(fcntl(fd, F_GETPATH, canonical_path)) != 0) { + if (HANDLE_EINTR(fcntl(fd.get(), F_GETPATH, canonical_path)) != 0) { DPLOG(FATAL) << "GetCanonicalSandboxPath() failed for: " << path.value(); return path; diff --git a/content/common/sandbox_mac_system_access_unittest.mm b/content/common/sandbox_mac_system_access_unittest.mm index 09ac854..4ddc73a 100644 --- a/content/common/sandbox_mac_system_access_unittest.mm +++ b/content/common/sandbox_mac_system_access_unittest.mm @@ -5,6 +5,7 @@ #import <Cocoa/Cocoa.h> #include "base/file_util.h" +#include "base/files/scoped_file.h" #include "base/logging.h" #include "base/strings/sys_string_conversions.h" #include "content/common/sandbox_mac.h" @@ -86,9 +87,8 @@ class MacSandboxedFileAccessTestCase : public MacSandboxTestCase { REGISTER_SANDBOX_TEST_CASE(MacSandboxedFileAccessTestCase); bool MacSandboxedFileAccessTestCase::SandboxedTest() { - int fdes = open("/etc/passwd", O_RDONLY); - file_util::ScopedFD file_closer(&fdes); - return fdes == -1; + base::ScopedFD fdes(open("/etc/passwd", O_RDONLY)); + return !fdes.is_valid(); } TEST_F(MacSandboxTest, FileAccess) { @@ -105,15 +105,14 @@ class MacSandboxedUrandomTestCase : public MacSandboxTestCase { REGISTER_SANDBOX_TEST_CASE(MacSandboxedUrandomTestCase); bool MacSandboxedUrandomTestCase::SandboxedTest() { - int fdes = open("/dev/urandom", O_RDONLY); - file_util::ScopedFD file_closer(&fdes); + base::ScopedFD fdes(open("/dev/urandom", O_RDONLY)); // Opening /dev/urandom succeeds under the sandbox. - if (fdes == -1) + if (!fdes.is_valid()) return false; char buf[16]; - int rc = read(fdes, buf, sizeof(buf)); + int rc = read(fdes.get(), buf, sizeof(buf)); return rc == sizeof(buf); } diff --git a/ipc/ipc_channel_factory.cc b/ipc/ipc_channel_factory.cc index 5c24284..244024c 100644 --- a/ipc/ipc_channel_factory.cc +++ b/ipc/ipc_channel_factory.cc @@ -5,6 +5,7 @@ #include "ipc/ipc_channel_factory.h" #include "base/file_util.h" +#include "base/files/scoped_file.h" #include "base/logging.h" #include "ipc/unix_domain_socket_util.h" @@ -51,21 +52,20 @@ void ChannelFactory::OnFileCanReadWithoutBlocking(int fd) { delegate_->OnListenError(); return; } + base::ScopedFD scoped_fd(new_fd); - if (new_fd < 0) { + if (!scoped_fd.is_valid()) { // The accept() failed, but not in such a way that the factory needs to be // shut down. return; } - file_util::ScopedFD scoped_fd(&new_fd); - // Verify that the IPC channel peer is running as the same user. - if (!IsPeerAuthorized(new_fd)) + if (!IsPeerAuthorized(scoped_fd.get())) return; ChannelHandle handle(std::string(), - base::FileDescriptor(*scoped_fd.release(), true)); + base::FileDescriptor(scoped_fd.release(), true)); delegate_->OnClientConnected(handle); } diff --git a/ipc/unix_domain_socket_util.cc b/ipc/unix_domain_socket_util.cc index 10df9b2..c29ce92 100644 --- a/ipc/unix_domain_socket_util.cc +++ b/ipc/unix_domain_socket_util.cc @@ -13,6 +13,7 @@ #include "base/file_util.h" #include "base/files/file_path.h" +#include "base/files/scoped_file.h" #include "base/logging.h" #include "base/posix/eintr_wrapper.h" @@ -45,15 +46,14 @@ int MakeUnixAddrForPath(const std::string& socket_name, } // Create socket. - int fd = socket(AF_UNIX, SOCK_STREAM, 0); - if (fd < 0) { + base::ScopedFD fd(socket(AF_UNIX, SOCK_STREAM, 0)); + if (!fd.is_valid()) { PLOG(ERROR) << "socket"; return -1; } - file_util::ScopedFD scoped_fd(&fd); // Make socket non-blocking - if (HANDLE_EINTR(fcntl(fd, F_SETFL, O_NONBLOCK)) < 0) { + if (HANDLE_EINTR(fcntl(fd.get(), F_SETFL, O_NONBLOCK)) < 0) { PLOG(ERROR) << "fcntl(O_NONBLOCK)"; return -1; } @@ -64,7 +64,7 @@ int MakeUnixAddrForPath(const std::string& socket_name, strncpy(unix_addr->sun_path, socket_name.c_str(), kMaxSocketNameLength); *unix_addr_len = offsetof(struct sockaddr_un, sun_path) + socket_name.length(); - return *scoped_fd.release(); + return fd.release(); } } // namespace @@ -78,10 +78,10 @@ bool CreateServerUnixDomainSocket(const base::FilePath& socket_path, struct sockaddr_un unix_addr; size_t unix_addr_len; - int fd = MakeUnixAddrForPath(socket_name, &unix_addr, &unix_addr_len); - if (fd < 0) + base::ScopedFD fd( + MakeUnixAddrForPath(socket_name, &unix_addr, &unix_addr_len)); + if (!fd.is_valid()) return false; - file_util::ScopedFD scoped_fd(&fd); // Make sure the path we need exists. if (!base::CreateDirectory(socket_dir)) { @@ -96,20 +96,20 @@ bool CreateServerUnixDomainSocket(const base::FilePath& socket_path, } // Bind the socket. - if (bind(fd, reinterpret_cast<const sockaddr*>(&unix_addr), + if (bind(fd.get(), reinterpret_cast<const sockaddr*>(&unix_addr), unix_addr_len) < 0) { PLOG(ERROR) << "bind " << socket_path.value(); return false; } // Start listening on the socket. - if (listen(fd, SOMAXCONN) < 0) { + if (listen(fd.get(), SOMAXCONN) < 0) { PLOG(ERROR) << "listen " << socket_path.value(); unlink(socket_name.c_str()); return false; } - *server_listen_fd = *scoped_fd.release(); + *server_listen_fd = fd.release(); return true; } @@ -122,18 +122,18 @@ bool CreateClientUnixDomainSocket(const base::FilePath& socket_path, struct sockaddr_un unix_addr; size_t unix_addr_len; - int fd = MakeUnixAddrForPath(socket_name, &unix_addr, &unix_addr_len); - if (fd < 0) + base::ScopedFD fd( + MakeUnixAddrForPath(socket_name, &unix_addr, &unix_addr_len)); + if (!fd.is_valid()) return false; - file_util::ScopedFD scoped_fd(&fd); - if (HANDLE_EINTR(connect(fd, reinterpret_cast<sockaddr*>(&unix_addr), + if (HANDLE_EINTR(connect(fd.get(), reinterpret_cast<sockaddr*>(&unix_addr), unix_addr_len)) < 0) { PLOG(ERROR) << "connect " << socket_path.value(); return false; } - *client_socket = *scoped_fd.release(); + *client_socket = fd.release(); return true; } @@ -184,18 +184,17 @@ bool ServerAcceptConnection(int server_listen_fd, int* server_socket) { DCHECK(server_socket); *server_socket = -1; - int accept_fd = HANDLE_EINTR(accept(server_listen_fd, NULL, 0)); - if (accept_fd < 0) + base::ScopedFD accept_fd(HANDLE_EINTR(accept(server_listen_fd, NULL, 0))); + if (!accept_fd.is_valid()) return IsRecoverableError(errno); - file_util::ScopedFD scoped_fd(&accept_fd); - if (HANDLE_EINTR(fcntl(accept_fd, F_SETFL, O_NONBLOCK)) < 0) { - PLOG(ERROR) << "fcntl(O_NONBLOCK) " << accept_fd; + if (HANDLE_EINTR(fcntl(accept_fd.get(), F_SETFL, O_NONBLOCK)) < 0) { + PLOG(ERROR) << "fcntl(O_NONBLOCK) " << accept_fd.get(); // It's safe to keep listening on |server_listen_fd| even if the attempt to // set O_NONBLOCK failed on the client fd. return true; } - *server_socket = *scoped_fd.release(); + *server_socket = accept_fd.release(); return true; } diff --git a/media/video/capture/linux/video_capture_device_linux.cc b/media/video/capture/linux/video_capture_device_linux.cc index fa3f83b..7a9ed69 100644 --- a/media/video/capture/linux/video_capture_device_linux.cc +++ b/media/video/capture/linux/video_capture_device_linux.cc @@ -19,6 +19,7 @@ #include "base/bind.h" #include "base/files/file_enumerator.h" +#include "base/files/scoped_file.h" #include "base/posix/eintr_wrapper.h" #include "base/strings/stringprintf.h" @@ -119,19 +120,18 @@ void VideoCaptureDevice::GetDeviceNames(Names* device_names) { base::FileEnumerator::FileInfo info = enumerator.GetInfo(); std::string unique_id = path.value() + info.GetName().value(); - int fd; - if ((fd = HANDLE_EINTR(open(unique_id.c_str() , O_RDONLY))) < 0) { + base::ScopedFD fd(HANDLE_EINTR(open(unique_id.c_str(), O_RDONLY))); + if (!fd.is_valid()) { // Failed to open this device. continue; } - file_util::ScopedFD fd_closer(&fd); // Test if this is a V4L2 capture device. v4l2_capability cap; - if ((HANDLE_EINTR(ioctl(fd, VIDIOC_QUERYCAP, &cap)) == 0) && + if ((HANDLE_EINTR(ioctl(fd.get(), VIDIOC_QUERYCAP, &cap)) == 0) && (cap.capabilities & V4L2_CAP_VIDEO_CAPTURE) && !(cap.capabilities & V4L2_CAP_VIDEO_OUTPUT)) { // This is a V4L2 video capture device - if (HasUsableFormats(fd)) { + if (HasUsableFormats(fd.get())) { Name device_name(base::StringPrintf("%s", cap.card), unique_id); device_names->push_back(device_name); } else { @@ -146,19 +146,18 @@ void VideoCaptureDevice::GetDeviceSupportedFormats( VideoCaptureFormats* supported_formats) { if (device.id().empty()) return; - int fd; - if ((fd = HANDLE_EINTR(open(device.id().c_str(), O_RDONLY))) < 0) { + base::ScopedFD fd(HANDLE_EINTR(open(device.id().c_str(), O_RDONLY))); + if (!fd.is_valid()) { // Failed to open this device. return; } - file_util::ScopedFD fd_closer(&fd); supported_formats->clear(); // Retrieve the caps one by one, first get pixel format, then sizes, then // frame rates. See http://linuxtv.org/downloads/v4l-dvb-apis for reference. v4l2_fmtdesc pixel_format = {}; pixel_format.type = V4L2_BUF_TYPE_VIDEO_CAPTURE; - while (HANDLE_EINTR(ioctl(fd, VIDIOC_ENUM_FMT, &pixel_format)) == 0) { + while (HANDLE_EINTR(ioctl(fd.get(), VIDIOC_ENUM_FMT, &pixel_format)) == 0) { VideoCaptureFormat supported_format; supported_format.pixel_format = V4l2ColorToVideoCaptureColorFormat((int32)pixel_format.pixelformat); @@ -169,7 +168,8 @@ void VideoCaptureDevice::GetDeviceSupportedFormats( v4l2_frmsizeenum frame_size = {}; frame_size.pixel_format = pixel_format.pixelformat; - while (HANDLE_EINTR(ioctl(fd, VIDIOC_ENUM_FRAMESIZES, &frame_size)) == 0) { + while (HANDLE_EINTR(ioctl(fd.get(), VIDIOC_ENUM_FRAMESIZES, &frame_size)) == + 0) { if (frame_size.type == V4L2_FRMSIZE_TYPE_DISCRETE) { supported_format.frame_size.SetSize( frame_size.discrete.width, frame_size.discrete.height); @@ -184,8 +184,8 @@ void VideoCaptureDevice::GetDeviceSupportedFormats( frame_interval.pixel_format = pixel_format.pixelformat; frame_interval.width = frame_size.discrete.width; frame_interval.height = frame_size.discrete.height; - while (HANDLE_EINTR( - ioctl(fd, VIDIOC_ENUM_FRAMEINTERVALS, &frame_interval)) == 0) { + while (HANDLE_EINTR(ioctl( + fd.get(), VIDIOC_ENUM_FRAMEINTERVALS, &frame_interval)) == 0) { if (frame_interval.type == V4L2_FRMIVAL_TYPE_DISCRETE) { if (frame_interval.discrete.numerator != 0) { supported_format.frame_rate = @@ -255,13 +255,12 @@ VideoCaptureDevice* VideoCaptureDevice::Create(const Name& device_name) { // Test opening the device driver. This is to make sure it is available. // We will reopen it again in our worker thread when someone // allocates the camera. - int fd = HANDLE_EINTR(open(device_name.id().c_str(), O_RDONLY)); - if (fd < 0) { + base::ScopedFD fd(HANDLE_EINTR(open(device_name.id().c_str(), O_RDONLY))); + if (!fd.is_valid()) { DVLOG(1) << "Cannot open device"; delete self; return NULL; } - close(fd); return self; } @@ -269,7 +268,6 @@ VideoCaptureDevice* VideoCaptureDevice::Create(const Name& device_name) { VideoCaptureDeviceLinux::VideoCaptureDeviceLinux(const Name& device_name) : state_(kIdle), device_name_(device_name), - device_fd_(-1), v4l2_thread_("V4L2Thread"), buffer_pool_(NULL), buffer_pool_size_(0), @@ -324,21 +322,19 @@ void VideoCaptureDeviceLinux::OnAllocateAndStart(int width, client_ = client.Pass(); // Need to open camera with O_RDWR after Linux kernel 3.3. - device_fd_ = HANDLE_EINTR(open(device_name_.id().c_str(), O_RDWR)); - if (device_fd_ < 0) { + device_fd_.reset(HANDLE_EINTR(open(device_name_.id().c_str(), O_RDWR))); + if (!device_fd_.is_valid()) { SetErrorState("Failed to open V4L2 device driver."); return; } - device_fd_closer_.reset(&device_fd_); // Test if this is a V4L2 capture device. v4l2_capability cap; - if (!((HANDLE_EINTR(ioctl(device_fd_, VIDIOC_QUERYCAP, &cap)) == 0) && + if (!((HANDLE_EINTR(ioctl(device_fd_.get(), VIDIOC_QUERYCAP, &cap)) == 0) && (cap.capabilities & V4L2_CAP_VIDEO_CAPTURE) && !(cap.capabilities & V4L2_CAP_VIDEO_OUTPUT))) { // This is not a V4L2 video capture device. - device_fd_closer_.reset(); - device_fd_ = -1; + device_fd_.reset(); SetErrorState("This is not a V4L2 video capture device"); return; } @@ -355,7 +351,8 @@ void VideoCaptureDeviceLinux::OnAllocateAndStart(int width, // Enumerate image formats. std::list<int>::iterator best = v4l2_formats.end(); - while (HANDLE_EINTR(ioctl(device_fd_, VIDIOC_ENUM_FMT, &fmtdesc)) == 0) { + while (HANDLE_EINTR(ioctl(device_fd_.get(), VIDIOC_ENUM_FMT, &fmtdesc)) == + 0) { best = std::find(v4l2_formats.begin(), best, fmtdesc.pixelformat); fmtdesc.index++; } @@ -374,7 +371,7 @@ void VideoCaptureDeviceLinux::OnAllocateAndStart(int width, video_fmt.fmt.pix.height = height; video_fmt.fmt.pix.pixelformat = *best; - if (HANDLE_EINTR(ioctl(device_fd_, VIDIOC_S_FMT, &video_fmt)) < 0) { + if (HANDLE_EINTR(ioctl(device_fd_.get(), VIDIOC_S_FMT, &video_fmt)) < 0) { SetErrorState("Failed to set camera format"); return; } @@ -384,14 +381,15 @@ void VideoCaptureDeviceLinux::OnAllocateAndStart(int width, memset(&streamparm, 0, sizeof(v4l2_streamparm)); streamparm.type = V4L2_BUF_TYPE_VIDEO_CAPTURE; // The following line checks that the driver knows about framerate get/set. - if (HANDLE_EINTR(ioctl(device_fd_, VIDIOC_G_PARM, &streamparm)) >= 0) { + if (HANDLE_EINTR(ioctl(device_fd_.get(), VIDIOC_G_PARM, &streamparm)) >= 0) { // Now check if the device is able to accept a capture framerate set. if (streamparm.parm.capture.capability & V4L2_CAP_TIMEPERFRAME) { streamparm.parm.capture.timeperframe.numerator = 1; streamparm.parm.capture.timeperframe.denominator = (frame_rate) ? frame_rate : kTypicalFramerate; - if (HANDLE_EINTR(ioctl(device_fd_, VIDIOC_S_PARM, &streamparm)) < 0) { + if (HANDLE_EINTR(ioctl(device_fd_.get(), VIDIOC_S_PARM, &streamparm)) < + 0) { SetErrorState("Failed to set camera framerate"); return; } @@ -419,7 +417,7 @@ void VideoCaptureDeviceLinux::OnAllocateAndStart(int width, // Start UVC camera. v4l2_buf_type type = V4L2_BUF_TYPE_VIDEO_CAPTURE; - if (HANDLE_EINTR(ioctl(device_fd_, VIDIOC_STREAMON, &type)) == -1) { + if (HANDLE_EINTR(ioctl(device_fd_.get(), VIDIOC_STREAMON, &type)) == -1) { SetErrorState("VIDIOC_STREAMON failed"); return; } @@ -436,7 +434,7 @@ void VideoCaptureDeviceLinux::OnStopAndDeAllocate() { DCHECK_EQ(v4l2_thread_.message_loop(), base::MessageLoop::current()); v4l2_buf_type type = V4L2_BUF_TYPE_VIDEO_CAPTURE; - if (HANDLE_EINTR(ioctl(device_fd_, VIDIOC_STREAMOFF, &type)) < 0) { + if (HANDLE_EINTR(ioctl(device_fd_.get(), VIDIOC_STREAMOFF, &type)) < 0) { SetErrorState("VIDIOC_STREAMOFF failed"); return; } @@ -447,8 +445,7 @@ void VideoCaptureDeviceLinux::OnStopAndDeAllocate() { // We need to close and open the device if we want to change the settings // Otherwise VIDIOC_S_FMT will return error // Sad but true. - device_fd_closer_.reset(); - device_fd_ = -1; + device_fd_.reset(); state_ = kIdle; client_.reset(); } @@ -462,7 +459,7 @@ void VideoCaptureDeviceLinux::OnCaptureTask() { fd_set r_set; FD_ZERO(&r_set); - FD_SET(device_fd_, &r_set); + FD_SET(device_fd_.get(), &r_set); timeval timeout; timeout.tv_sec = 0; @@ -471,7 +468,7 @@ void VideoCaptureDeviceLinux::OnCaptureTask() { // First argument to select is the highest numbered file descriptor +1. // Refer to http://linux.die.net/man/2/select for more information. int result = - HANDLE_EINTR(select(device_fd_ + 1, &r_set, NULL, NULL, &timeout)); + HANDLE_EINTR(select(device_fd_.get() + 1, &r_set, NULL, NULL, &timeout)); // Check if select have failed. if (result < 0) { // EINTR is a signal. This is not really an error. @@ -500,13 +497,13 @@ void VideoCaptureDeviceLinux::OnCaptureTask() { } // Check if the driver have filled a buffer. - if (FD_ISSET(device_fd_, &r_set)) { + if (FD_ISSET(device_fd_.get(), &r_set)) { v4l2_buffer buffer; memset(&buffer, 0, sizeof(buffer)); buffer.type = V4L2_BUF_TYPE_VIDEO_CAPTURE; buffer.memory = V4L2_MEMORY_MMAP; // Dequeue a buffer. - if (HANDLE_EINTR(ioctl(device_fd_, VIDIOC_DQBUF, &buffer)) == 0) { + if (HANDLE_EINTR(ioctl(device_fd_.get(), VIDIOC_DQBUF, &buffer)) == 0) { client_->OnIncomingCapturedData( static_cast<uint8*>(buffer_pool_[buffer.index].start), buffer.bytesused, @@ -515,7 +512,7 @@ void VideoCaptureDeviceLinux::OnCaptureTask() { base::TimeTicks::Now()); // Enqueue the buffer again. - if (HANDLE_EINTR(ioctl(device_fd_, VIDIOC_QBUF, &buffer)) == -1) { + if (HANDLE_EINTR(ioctl(device_fd_.get(), VIDIOC_QBUF, &buffer)) == -1) { SetErrorState(base::StringPrintf( "Failed to enqueue capture buffer errno %d", errno)); } @@ -540,7 +537,7 @@ bool VideoCaptureDeviceLinux::AllocateVideoBuffers() { r_buffer.memory = V4L2_MEMORY_MMAP; r_buffer.count = kMaxVideoBuffers; - if (HANDLE_EINTR(ioctl(device_fd_, VIDIOC_REQBUFS, &r_buffer)) < 0) { + if (HANDLE_EINTR(ioctl(device_fd_.get(), VIDIOC_REQBUFS, &r_buffer)) < 0) { return false; } @@ -559,20 +556,20 @@ bool VideoCaptureDeviceLinux::AllocateVideoBuffers() { buffer.memory = V4L2_MEMORY_MMAP; buffer.index = i; - if (HANDLE_EINTR(ioctl(device_fd_, VIDIOC_QUERYBUF, &buffer)) < 0) { + if (HANDLE_EINTR(ioctl(device_fd_.get(), VIDIOC_QUERYBUF, &buffer)) < 0) { return false; } // Some devices require mmap() to be called with both READ and WRITE. // See crbug.com/178582. buffer_pool_[i].start = mmap(NULL, buffer.length, PROT_READ | PROT_WRITE, - MAP_SHARED, device_fd_, buffer.m.offset); + MAP_SHARED, device_fd_.get(), buffer.m.offset); if (buffer_pool_[i].start == MAP_FAILED) { return false; } buffer_pool_[i].length = buffer.length; // Enqueue the buffer in the drivers incoming queue. - if (HANDLE_EINTR(ioctl(device_fd_, VIDIOC_QBUF, &buffer)) < 0) { + if (HANDLE_EINTR(ioctl(device_fd_.get(), VIDIOC_QBUF, &buffer)) < 0) { return false; } } @@ -593,7 +590,7 @@ void VideoCaptureDeviceLinux::DeAllocateVideoBuffers() { r_buffer.memory = V4L2_MEMORY_MMAP; r_buffer.count = 0; - if (HANDLE_EINTR(ioctl(device_fd_, VIDIOC_REQBUFS, &r_buffer)) < 0) { + if (HANDLE_EINTR(ioctl(device_fd_.get(), VIDIOC_REQBUFS, &r_buffer)) < 0) { SetErrorState("Failed to reset buf."); } diff --git a/media/video/capture/linux/video_capture_device_linux.h b/media/video/capture/linux/video_capture_device_linux.h index 4cc1597..6a8dcee 100644 --- a/media/video/capture/linux/video_capture_device_linux.h +++ b/media/video/capture/linux/video_capture_device_linux.h @@ -13,6 +13,7 @@ #include <string> #include "base/file_util.h" +#include "base/files/scoped_file.h" #include "base/threading/thread.h" #include "media/video/capture/video_capture_device.h" #include "media/video/capture/video_capture_types.h" @@ -60,8 +61,7 @@ class VideoCaptureDeviceLinux : public VideoCaptureDevice { InternalState state_; scoped_ptr<VideoCaptureDevice::Client> client_; Name device_name_; - int device_fd_; // File descriptor for the opened camera device. - file_util::ScopedFD device_fd_closer_; + base::ScopedFD device_fd_; // File descriptor for the opened camera device. base::Thread v4l2_thread_; // Thread used for reading data from the device. Buffer* buffer_pool_; int buffer_pool_size_; // Number of allocated buffers. diff --git a/net/test/spawned_test_server/local_test_server.h b/net/test/spawned_test_server/local_test_server.h index cfd2eb3..d916835 100644 --- a/net/test/spawned_test_server/local_test_server.h +++ b/net/test/spawned_test_server/local_test_server.h @@ -8,6 +8,7 @@ #include <string> #include "base/file_util.h" +#include "base/files/scoped_file.h" #include "base/process/process_handle.h" #include "net/test/spawned_test_server/base_test_server.h" @@ -105,8 +106,7 @@ class LocalTestServer : public BaseTestServer { #if defined(OS_POSIX) // The file descriptor the child writes to when it starts. - int child_fd_; - file_util::ScopedFD child_fd_closer_; + base::ScopedFD child_fd_; #endif DISALLOW_COPY_AND_ASSIGN(LocalTestServer); diff --git a/net/test/spawned_test_server/local_test_server_posix.cc b/net/test/spawned_test_server/local_test_server_posix.cc index 10c2d0f..ea9cab6 100644 --- a/net/test/spawned_test_server/local_test_server_posix.cc +++ b/net/test/spawned_test_server/local_test_server_posix.cc @@ -10,6 +10,7 @@ #include "base/command_line.h" #include "base/file_util.h" +#include "base/files/scoped_file.h" #include "base/logging.h" #include "base/process/kill.h" #include "base/process/launch.h" @@ -120,9 +121,8 @@ bool LocalTestServer::LaunchPython(const base::FilePath& testserver_path) { } // Save the read half. The write half is sent to the child. - child_fd_ = pipefd[0]; - child_fd_closer_.reset(&child_fd_); - file_util::ScopedFD write_closer(&pipefd[1]); + child_fd_.reset(pipefd[0]); + base::ScopedFD write_closer(pipefd[1]); base::FileHandleMappingVector map_write_fd; map_write_fd.push_back(std::make_pair(pipefd[1], pipefd[1])); @@ -148,19 +148,19 @@ bool LocalTestServer::LaunchPython(const base::FilePath& testserver_path) { } bool LocalTestServer::WaitToStart() { - file_util::ScopedFD child_fd_closer(child_fd_closer_.release()); + base::ScopedFD our_fd(child_fd_.release()); base::TimeDelta remaining_time = TestTimeouts::action_timeout(); uint32 server_data_len = 0; - if (!ReadData(child_fd_, sizeof(server_data_len), + if (!ReadData(our_fd.get(), sizeof(server_data_len), reinterpret_cast<uint8*>(&server_data_len), &remaining_time)) { LOG(ERROR) << "Could not read server_data_len"; return false; } std::string server_data(server_data_len, '\0'); - if (!ReadData(child_fd_, server_data_len, + if (!ReadData(our_fd.get(), server_data_len, reinterpret_cast<uint8*>(&server_data[0]), &remaining_time)) { LOG(ERROR) << "Could not read server_data (" << server_data_len diff --git a/sandbox/linux/services/broker_process_unittest.cc b/sandbox/linux/services/broker_process_unittest.cc index 771790a..7f1a685 100644 --- a/sandbox/linux/services/broker_process_unittest.cc +++ b/sandbox/linux/services/broker_process_unittest.cc @@ -17,6 +17,7 @@ #include "base/basictypes.h" #include "base/bind.h" #include "base/file_util.h" +#include "base/files/scoped_file.h" #include "base/logging.h" #include "base/memory/scoped_ptr.h" #include "base/posix/eintr_wrapper.h" @@ -24,8 +25,6 @@ #include "sandbox/linux/tests/unit_tests.h" #include "testing/gtest/include/gtest/gtest.h" -using file_util::ScopedFD; - namespace sandbox { namespace { @@ -269,7 +268,7 @@ void TestOpenCpuinfo(bool fast_check_in_client) { int fd = -1; fd = open_broker->Open(kFileCpuInfo, O_RDWR); - ScopedFD fd_closer(&fd); + base::ScopedFD fd_closer(fd); ASSERT_EQ(fd, -EPERM); // Check we can read /proc/cpuinfo. @@ -281,7 +280,7 @@ void TestOpenCpuinfo(bool fast_check_in_client) { // Open cpuinfo via the broker. int cpuinfo_fd = open_broker->Open(kFileCpuInfo, O_RDONLY); - ScopedFD cpuinfo_fd_closer(&cpuinfo_fd); + base::ScopedFD cpuinfo_fd_closer(cpuinfo_fd); ASSERT_GE(cpuinfo_fd, 0); char buf[3]; memset(buf, 0, sizeof(buf)); @@ -290,7 +289,7 @@ void TestOpenCpuinfo(bool fast_check_in_client) { // Open cpuinfo directly. int cpuinfo_fd2 = open(kFileCpuInfo, O_RDONLY); - ScopedFD cpuinfo_fd2_closer(&cpuinfo_fd2); + base::ScopedFD cpuinfo_fd2_closer(cpuinfo_fd2); ASSERT_GE(cpuinfo_fd2, 0); char buf2[3]; memset(buf2, 1, sizeof(buf2)); diff --git a/sandbox/linux/services/credentials_unittest.cc b/sandbox/linux/services/credentials_unittest.cc index 9b792aa..a54ed04 100644 --- a/sandbox/linux/services/credentials_unittest.cc +++ b/sandbox/linux/services/credentials_unittest.cc @@ -12,13 +12,12 @@ #include <unistd.h> #include "base/file_util.h" +#include "base/files/scoped_file.h" #include "base/logging.h" #include "base/memory/scoped_ptr.h" #include "sandbox/linux/tests/unit_tests.h" #include "testing/gtest/include/gtest/gtest.h" -using file_util::ScopedFD; - namespace sandbox { namespace { @@ -65,7 +64,7 @@ TEST(Credentials, HasOpenDirectory) { { // Have a "/dev" file descriptor around. int dev_fd = open("/dev", O_RDONLY | O_DIRECTORY); - ScopedFD dev_fd_closer(&dev_fd); + base::ScopedFD dev_fd_closer(dev_fd); EXPECT_TRUE(creds.HasOpenDirectory(-1)); } EXPECT_FALSE(creds.HasOpenDirectory(-1)); @@ -75,7 +74,7 @@ TEST(Credentials, HasOpenDirectoryWithFD) { Credentials creds; int proc_fd = open("/proc", O_RDONLY | O_DIRECTORY); - ScopedFD proc_fd_closer(&proc_fd); + base::ScopedFD proc_fd_closer(proc_fd); ASSERT_LE(0, proc_fd); // Don't pass |proc_fd|, an open directory (proc_fd) should @@ -87,7 +86,7 @@ TEST(Credentials, HasOpenDirectoryWithFD) { { // Have a "/dev" file descriptor around. int dev_fd = open("/dev", O_RDONLY | O_DIRECTORY); - ScopedFD dev_fd_closer(&dev_fd); + base::ScopedFD dev_fd_closer(dev_fd); EXPECT_TRUE(creds.HasOpenDirectory(proc_fd)); } diff --git a/sandbox/linux/services/scoped_process_unittest.cc b/sandbox/linux/services/scoped_process_unittest.cc index 7ae82bf8..2800bd7 100644 --- a/sandbox/linux/services/scoped_process_unittest.cc +++ b/sandbox/linux/services/scoped_process_unittest.cc @@ -13,6 +13,7 @@ #include "base/bind.h" #include "base/callback.h" #include "base/file_util.h" +#include "base/files/scoped_file.h" #include "base/logging.h" #include "base/posix/eintr_wrapper.h" #include "base/threading/platform_thread.h" @@ -73,13 +74,13 @@ TEST(ScopedProcess, ScopedProcessSignaled) { TEST(ScopedProcess, DiesForReal) { int pipe_fds[2]; ASSERT_EQ(0, pipe(pipe_fds)); - file_util::ScopedFDCloser read_end_closer(pipe_fds); - file_util::ScopedFDCloser write_end_closer(pipe_fds + 1); + base::ScopedFD read_end_closer(pipe_fds[0]); + base::ScopedFD write_end_closer(pipe_fds[1]); { ScopedProcess process(base::Bind(&DoExit)); } // Close writing end of the pipe. - ASSERT_EQ(0, IGNORE_EINTR(close(pipe_fds[1]))); + write_end_closer.reset(); pipe_fds[1] = -1; ASSERT_EQ(0, fcntl(pipe_fds[0], F_SETFL, O_NONBLOCK)); @@ -108,8 +109,8 @@ void SleepInMsAndWriteOneByte(int time_to_sleep, int fd) { TEST(ScopedProcess, SynchronizationWorks) { int pipe_fds[2]; ASSERT_EQ(0, pipe(pipe_fds)); - file_util::ScopedFDCloser read_end_closer(pipe_fds); - file_util::ScopedFDCloser write_end_closer(pipe_fds + 1); + base::ScopedFD read_end_closer(pipe_fds[0]); + base::ScopedFD write_end_closer(pipe_fds[1]); // Start a process with a closure that takes a little bit to run. ScopedProcess process( diff --git a/sandbox/linux/services/yama.cc b/sandbox/linux/services/yama.cc index efb261c..39ac079 100644 --- a/sandbox/linux/services/yama.cc +++ b/sandbox/linux/services/yama.cc @@ -12,6 +12,7 @@ #include "base/basictypes.h" #include "base/file_util.h" +#include "base/files/scoped_file.h" #include "base/logging.h" #include "base/posix/eintr_wrapper.h" @@ -78,18 +79,17 @@ int Yama::GetStatus() { static const char kPtraceScopePath[] = "/proc/sys/kernel/yama/ptrace_scope"; - int yama_scope = open(kPtraceScopePath, O_RDONLY); + base::ScopedFD yama_scope(open(kPtraceScopePath, O_RDONLY)); - if (yama_scope < 0) { + if (!yama_scope.is_valid()) { const int open_errno = errno; DCHECK(ENOENT == open_errno); // The status is known, yama is not present. return STATUS_KNOWN; } - file_util::ScopedFDCloser yama_scope_closer(&yama_scope); char yama_scope_value = 0; - ssize_t num_read = read(yama_scope, &yama_scope_value, 1); + ssize_t num_read = read(yama_scope.get(), &yama_scope_value, 1); PCHECK(1 == num_read); switch (yama_scope_value) { diff --git a/tools/android/memdump/memdump.cc b/tools/android/memdump/memdump.cc index 20684f6..4f4dd14 100644 --- a/tools/android/memdump/memdump.cc +++ b/tools/android/memdump/memdump.cc @@ -22,6 +22,7 @@ #include "base/callback_helpers.h" #include "base/containers/hash_tables.h" #include "base/file_util.h" +#include "base/files/scoped_file.h" #include "base/logging.h" #include "base/strings/string_number_conversions.h" #include "base/strings/string_split.h" @@ -436,13 +437,12 @@ bool CollectProcessMemoryInformation(int page_count_fd, int page_flags_fd, ProcessMemory* process_memory) { const pid_t pid = process_memory->pid; - int pagemap_fd = open( - base::StringPrintf("/proc/%d/pagemap", pid).c_str(), O_RDONLY); - if (pagemap_fd < 0) { + base::ScopedFD pagemap_fd(open( + base::StringPrintf("/proc/%d/pagemap", pid).c_str(), O_RDONLY)); + if (!pagemap_fd.is_valid()) { PLOG(ERROR) << "open"; return false; } - file_util::ScopedFD auto_closer(&pagemap_fd); std::vector<MemoryMap>* const process_maps = &process_memory->memory_maps; if (!GetProcessMaps(pid, process_maps)) return false; @@ -450,7 +450,7 @@ bool CollectProcessMemoryInformation(int page_count_fd, it != process_maps->end(); ++it) { std::vector<PageInfo>* const committed_pages = &it->committed_pages; BitSet* const pages_bits = &it->committed_pages_bits; - GetPagesForMemoryMap(pagemap_fd, *it, committed_pages, pages_bits); + GetPagesForMemoryMap(pagemap_fd.get(), *it, committed_pages, pages_bits); SetPagesInfo(page_count_fd, page_flags_fd, committed_pages); } return true; @@ -489,21 +489,18 @@ int main(int argc, char** argv) { std::vector<ProcessMemory> processes_memory(pids.size()); { - int page_count_fd = open("/proc/kpagecount", O_RDONLY); - if (page_count_fd < 0) { + base::ScopedFD page_count_fd(open("/proc/kpagecount", O_RDONLY)); + if (!page_count_fd.is_valid()) { PLOG(ERROR) << "open /proc/kpagecount"; return EXIT_FAILURE; } - int page_flags_fd = open("/proc/kpageflags", O_RDONLY); - if (page_flags_fd < 0) { + base::ScopedFD page_flags_fd(open("/proc/kpageflags", O_RDONLY)); + if (!page_flags_fd.is_valid()) { PLOG(ERROR) << "open /proc/kpageflags"; return EXIT_FAILURE; } - file_util::ScopedFD page_count_fd_closer(&page_count_fd); - file_util::ScopedFD page_flags_fd_closer(&page_flags_fd); - base::ScopedClosureRunner auto_resume_processes( base::Bind(&KillAll, pids, SIGCONT)); KillAll(pids, SIGSTOP); @@ -513,7 +510,7 @@ int main(int argc, char** argv) { &processes_memory[it - pids.begin()]; process_memory->pid = *it; if (!CollectProcessMemoryInformation( - page_count_fd, page_flags_fd, process_memory)) { + page_count_fd.get(), page_flags_fd.get(), process_memory)) { return EXIT_FAILURE; } } |