diff --git a/client/crash_report_database_test.cc b/client/crash_report_database_test.cc index 6a04d19c..5e728d92 100644 --- a/client/crash_report_database_test.cc +++ b/client/crash_report_database_test.cc @@ -14,12 +14,11 @@ #include "client/crash_report_database.h" -#include - #include "build/build_config.h" #include "client/settings.h" #include "gtest/gtest.h" #include "test/errors.h" +#include "test/file.h" #include "test/scoped_temp_dir.h" #include "util/file/file_io.h" @@ -27,18 +26,6 @@ namespace crashpad { namespace test { namespace { -bool FileExistsAtPath(const base::FilePath& path) { -#if defined(OS_POSIX) - struct stat st; - return lstat(path.value().c_str(), &st) == 0; -#elif defined(OS_WIN) - struct _stat st; - return _wstat(path.value().c_str(), &st) == 0; -#else -#error "Not implemented" -#endif -} - class CrashReportDatabaseTest : public testing::Test { public: CrashReportDatabaseTest() { @@ -74,7 +61,7 @@ class CrashReportDatabaseTest : public testing::Test { EXPECT_EQ(CrashReportDatabase::kNoError, db_->LookUpCrashReport(uuid, report)); ExpectPreparedCrashReport(*report); - ASSERT_TRUE(FileExistsAtPath(report->file_path)); + ASSERT_TRUE(FileExists(report->file_path)); } void UploadReport(const UUID& uuid, bool successful, const std::string& id) { @@ -92,7 +79,7 @@ class CrashReportDatabaseTest : public testing::Test { db_->GetReportForUploading(uuid, &report)); EXPECT_NE(UUID(), report->uuid); EXPECT_FALSE(report->file_path.empty()); - EXPECT_TRUE(FileExistsAtPath(report->file_path)) + EXPECT_TRUE(FileExists(report->file_path)) << report->file_path.value(); EXPECT_GT(report->creation_time, 0); EXPECT_EQ(CrashReportDatabase::kNoError, @@ -110,7 +97,7 @@ class CrashReportDatabaseTest : public testing::Test { void ExpectPreparedCrashReport(const CrashReportDatabase::Report& report) { EXPECT_NE(UUID(), report.uuid); EXPECT_FALSE(report.file_path.empty()); - EXPECT_TRUE(FileExistsAtPath(report.file_path)) << report.file_path.value(); + EXPECT_TRUE(FileExists(report.file_path)) << report.file_path.value(); EXPECT_TRUE(report.id.empty()); EXPECT_GT(report.creation_time, 0); EXPECT_FALSE(report.uploaded); @@ -180,7 +167,7 @@ TEST_F(CrashReportDatabaseTest, NewCrashReport) { EXPECT_EQ(CrashReportDatabase::kNoError, db()->PrepareNewCrashReport(&new_report)); UUID expect_uuid = new_report->uuid; - EXPECT_TRUE(FileExistsAtPath(new_report->path)) << new_report->path.value(); + EXPECT_TRUE(FileExists(new_report->path)) << new_report->path.value(); UUID uuid; EXPECT_EQ(CrashReportDatabase::kNoError, db()->FinishedWritingCrashReport(new_report, &uuid)); @@ -208,10 +195,10 @@ TEST_F(CrashReportDatabaseTest, ErrorWritingCrashReport) { ASSERT_EQ(CrashReportDatabase::kNoError, db()->PrepareNewCrashReport(&new_report)); base::FilePath new_report_path = new_report->path; - EXPECT_TRUE(FileExistsAtPath(new_report_path)) << new_report_path.value(); + EXPECT_TRUE(FileExists(new_report_path)) << new_report_path.value(); EXPECT_EQ(CrashReportDatabase::kNoError, db()->ErrorWritingCrashReport(new_report)); - EXPECT_FALSE(FileExistsAtPath(new_report_path)) << new_report_path.value(); + EXPECT_FALSE(FileExists(new_report_path)) << new_report_path.value(); } TEST_F(CrashReportDatabaseTest, LookUpCrashReport) { @@ -488,7 +475,7 @@ TEST_F(CrashReportDatabaseTest, MoveDatabase) { CrashReportDatabase::NewReport* new_report; EXPECT_EQ(CrashReportDatabase::kNoError, db()->PrepareNewCrashReport(&new_report)); - EXPECT_TRUE(FileExistsAtPath(new_report->path)) << new_report->path.value(); + EXPECT_TRUE(FileExists(new_report->path)) << new_report->path.value(); UUID uuid; EXPECT_EQ(CrashReportDatabase::kNoError, db()->FinishedWritingCrashReport(new_report, &uuid)); @@ -499,14 +486,14 @@ TEST_F(CrashReportDatabaseTest, MoveDatabase) { EXPECT_EQ(CrashReportDatabase::kNoError, db()->LookUpCrashReport(uuid, &report)); ExpectPreparedCrashReport(report); - EXPECT_TRUE(FileExistsAtPath(report.file_path)) << report.file_path.value(); + EXPECT_TRUE(FileExists(report.file_path)) << report.file_path.value(); } TEST_F(CrashReportDatabaseTest, ReportRemoved) { CrashReportDatabase::NewReport* new_report; EXPECT_EQ(CrashReportDatabase::kNoError, db()->PrepareNewCrashReport(&new_report)); - EXPECT_TRUE(FileExistsAtPath(new_report->path)) << new_report->path.value(); + EXPECT_TRUE(FileExists(new_report->path)) << new_report->path.value(); UUID uuid; EXPECT_EQ(CrashReportDatabase::kNoError, db()->FinishedWritingCrashReport(new_report, &uuid)); diff --git a/test/file.cc b/test/file.cc new file mode 100644 index 00000000..9c007433 --- /dev/null +++ b/test/file.cc @@ -0,0 +1,66 @@ +// Copyright 2015 The Crashpad Authors. All rights reserved. +// +// Licensed under the Apache License, Version 2.0 (the "License"); +// you may not use this file except in compliance with the License. +// You may obtain a copy of the License at +// +// http://www.apache.org/licenses/LICENSE-2.0 +// +// Unless required by applicable law or agreed to in writing, software +// distributed under the License is distributed on an "AS IS" BASIS, +// WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied. +// See the License for the specific language governing permissions and +// limitations under the License. + +#include "test/file.h" + +#include +#include + +#include "gtest/gtest.h" +#include "test/errors.h" + +namespace crashpad { +namespace test { + +bool FileExists(const base::FilePath& path) { +#if defined(OS_POSIX) + struct stat st; + int rv = lstat(path.value().c_str(), &st); + const char stat_function[] = "lstat"; +#elif defined(OS_WIN) + struct _stat st; + int rv = _wstat(path.value().c_str(), &st); + const char stat_function[] = "_wstat"; +#else +#error "Not implemented" +#endif + if (rv < 0) { + EXPECT_EQ(ENOENT, errno) << ErrnoMessage(stat_function) << " " + << path.value(); + return false; + } + return true; +} + +FileOffset FileSize(const base::FilePath& path) { +#if defined(OS_POSIX) + struct stat st; + int rv = lstat(path.value().c_str(), &st); + const char stat_function[] = "lstat"; +#elif defined(OS_WIN) + struct _stati64 st; + int rv = _wstati64(path.value().c_str(), &st); + const char stat_function[] = "_wstati64"; +#else +#error "Not implemented" +#endif + if (rv < 0) { + ADD_FAILURE() << ErrnoMessage(stat_function) << " " << path.value(); + return -1; + } + return st.st_size; +} + +} // namespace test +} // namespace crashpad diff --git a/test/file.h b/test/file.h new file mode 100644 index 00000000..a9987b4d --- /dev/null +++ b/test/file.h @@ -0,0 +1,45 @@ +// Copyright 2015 The Crashpad Authors. All rights reserved. +// +// Licensed under the Apache License, Version 2.0 (the "License"); +// you may not use this file except in compliance with the License. +// You may obtain a copy of the License at +// +// http://www.apache.org/licenses/LICENSE-2.0 +// +// Unless required by applicable law or agreed to in writing, software +// distributed under the License is distributed on an "AS IS" BASIS, +// WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied. +// See the License for the specific language governing permissions and +// limitations under the License. + +#ifndef CRASHPAD_TEST_FILE_H_ +#define CRASHPAD_TEST_FILE_H_ + +#include "base/files/file_path.h" +#include "util/file/file_io.h" + +namespace crashpad { +namespace test { + +//! \brief Determines whether a file exists. +//! +//! \param[in] path The path to check for existence. +//! +//! \return `true` if \a path exists. `false` if it does not exist. If an error +//! other than “file not found” occurs when searching for \a path, returns +//! `false` with a gtest failure added. +bool FileExists(const base::FilePath& path); + +//! \brief Determines the size of a file. +//! +//! \param[in] path The path of the file to check. The file must exist. +//! +//! \return The size of the file at \a path. If the file does not exist, or an +//! error occurs when attempting to determine its size, returns `-1` with a +//! gtest failure added. +FileOffset FileSize(const base::FilePath& path); + +} // namespace test +} // namespace crashpad + +#endif // CRASHPAD_TEST_FILE_H_ diff --git a/test/scoped_temp_dir_test.cc b/test/scoped_temp_dir_test.cc index 6f3ce3a3..ea8b067b 100644 --- a/test/scoped_temp_dir_test.cc +++ b/test/scoped_temp_dir_test.cc @@ -14,15 +14,14 @@ #include "test/scoped_temp_dir.h" -#include #include #include -#include #include "base/posix/eintr_wrapper.h" #include "build/build_config.h" #include "gtest/gtest.h" #include "test/errors.h" +#include "test/file.h" #if defined(OS_POSIX) #include @@ -35,26 +34,6 @@ namespace crashpad { namespace test { namespace { -bool FileExists(const base::FilePath& path) { -#if defined(OS_POSIX) - struct stat st; - int rv = lstat(path.value().c_str(), &st); - const char stat_function[] = "lstat"; -#elif defined(OS_WIN) - struct _stat st; - int rv = _wstat(path.value().c_str(), &st); - const char stat_function[] = "_wstat"; -#else -#error "Not implemented" -#endif - if (rv < 0) { - EXPECT_EQ(ENOENT, errno) << ErrnoMessage(stat_function) << " " - << path.value(); - return false; - } - return true; -} - void CreateFile(const base::FilePath& path) { #if defined(OS_POSIX) int fd = HANDLE_EINTR(creat(path.value().c_str(), 0644)); diff --git a/test/test.gyp b/test/test.gyp index 6b290f8d..e8c1709c 100644 --- a/test/test.gyp +++ b/test/test.gyp @@ -32,6 +32,8 @@ 'sources': [ 'errors.cc', 'errors.h', + 'file.cc', + 'file.h', 'gtest_death_check.h', 'mac/dyld.h', 'mac/mach_errors.cc', diff --git a/util/file/file_io.h b/util/file/file_io.h index 0be3563f..7bc96735 100644 --- a/util/file/file_io.h +++ b/util/file/file_io.h @@ -79,6 +79,9 @@ const FileHandle kInvalidFileHandle = INVALID_HANDLE_VALUE; //! \brief Determines the mode that LoggingOpenFileForWrite() uses. enum class FileWriteMode { + //! \brief Opens the file if it exists, or fails if it does not. + kReuseOrFail, + //! \brief Opens the file if it exists, or creates a new file. kReuseOrCreate, diff --git a/util/file/file_io_posix.cc b/util/file/file_io_posix.cc index 9511b34e..29cdfec5 100644 --- a/util/file/file_io_posix.cc +++ b/util/file/file_io_posix.cc @@ -75,12 +75,22 @@ FileHandle LoggingOpenFileForOutput(int rdwr_or_wronly, const base::FilePath& path, FileWriteMode mode, FilePermissions permissions) { - int flags = rdwr_or_wronly | O_CREAT; - // kReuseOrCreate does not need any additional flags. - if (mode == FileWriteMode::kTruncateOrCreate) - flags |= O_TRUNC; - else if (mode == FileWriteMode::kCreateOrFail) - flags |= O_EXCL; + int flags = rdwr_or_wronly & (O_RDWR | O_WRONLY); + DCHECK_NE(flags, 0); + + switch (mode) { + case FileWriteMode::kReuseOrFail: + break; + case FileWriteMode::kReuseOrCreate: + flags |= O_CREAT; + break; + case FileWriteMode::kTruncateOrCreate: + flags |= O_CREAT | O_TRUNC; + break; + case FileWriteMode::kCreateOrFail: + flags |= O_CREAT | O_EXCL; + break; + } int fd = HANDLE_EINTR( open(path.value().c_str(), diff --git a/util/file/file_io_test.cc b/util/file/file_io_test.cc index 5719dccb..29cf3204 100644 --- a/util/file/file_io_test.cc +++ b/util/file/file_io_test.cc @@ -19,13 +19,100 @@ #include "base/files/file_path.h" #include "gtest/gtest.h" #include "test/errors.h" +#include "test/file.h" #include "test/scoped_temp_dir.h" +#include "util/misc/implicit_cast.h" #include "util/thread/thread.h" namespace crashpad { namespace test { namespace { +TEST(FileIO, OpenFileForWrite) { + ScopedTempDir temp_dir; + base::FilePath file_path_1 = + temp_dir.path().Append(FILE_PATH_LITERAL("file_1")); + ASSERT_FALSE(FileExists(file_path_1)); + + ScopedFileHandle + file_handle(LoggingOpenFileForWrite(file_path_1, + FileWriteMode::kReuseOrFail, + FilePermissions::kWorldReadable)); + EXPECT_EQ(kInvalidFileHandle, file_handle); + EXPECT_FALSE(FileExists(file_path_1)); + + file_handle.reset(LoggingOpenFileForWrite(file_path_1, + FileWriteMode::kCreateOrFail, + FilePermissions::kWorldReadable)); + EXPECT_NE(kInvalidFileHandle, file_handle); + EXPECT_TRUE(FileExists(file_path_1)); + EXPECT_EQ(0, FileSize(file_path_1)); + + file_handle.reset(LoggingOpenFileForWrite(file_path_1, + FileWriteMode::kReuseOrCreate, + FilePermissions::kWorldReadable)); + EXPECT_NE(kInvalidFileHandle, file_handle); + EXPECT_TRUE(FileExists(file_path_1)); + EXPECT_EQ(0, FileSize(file_path_1)); + + const char data = '%'; + EXPECT_TRUE(LoggingWriteFile(file_handle.get(), &data, sizeof(data))); + + // Close file_handle to ensure that the write is flushed to disk. + file_handle.reset(); + EXPECT_EQ(implicit_cast(sizeof(data)), FileSize(file_path_1)); + + file_handle.reset(LoggingOpenFileForWrite(file_path_1, + FileWriteMode::kReuseOrCreate, + FilePermissions::kWorldReadable)); + EXPECT_NE(kInvalidFileHandle, file_handle); + EXPECT_TRUE(FileExists(file_path_1)); + EXPECT_EQ(implicit_cast(sizeof(data)), FileSize(file_path_1)); + + file_handle.reset(LoggingOpenFileForWrite(file_path_1, + FileWriteMode::kCreateOrFail, + FilePermissions::kWorldReadable)); + EXPECT_EQ(kInvalidFileHandle, file_handle); + EXPECT_TRUE(FileExists(file_path_1)); + EXPECT_EQ(implicit_cast(sizeof(data)), FileSize(file_path_1)); + + file_handle.reset(LoggingOpenFileForWrite(file_path_1, + FileWriteMode::kReuseOrFail, + FilePermissions::kWorldReadable)); + EXPECT_NE(kInvalidFileHandle, file_handle); + EXPECT_TRUE(FileExists(file_path_1)); + EXPECT_EQ(implicit_cast(sizeof(data)), FileSize(file_path_1)); + + file_handle.reset(LoggingOpenFileForWrite(file_path_1, + FileWriteMode::kTruncateOrCreate, + FilePermissions::kWorldReadable)); + EXPECT_NE(kInvalidFileHandle, file_handle); + EXPECT_TRUE(FileExists(file_path_1)); + EXPECT_EQ(0, FileSize(file_path_1)); + + base::FilePath file_path_2 = + temp_dir.path().Append(FILE_PATH_LITERAL("file_2")); + ASSERT_FALSE(FileExists(file_path_2)); + + file_handle.reset(LoggingOpenFileForWrite(file_path_2, + FileWriteMode::kTruncateOrCreate, + FilePermissions::kWorldReadable)); + EXPECT_NE(kInvalidFileHandle, file_handle); + EXPECT_TRUE(FileExists(file_path_2)); + EXPECT_EQ(0, FileSize(file_path_2)); + + base::FilePath file_path_3 = + temp_dir.path().Append(FILE_PATH_LITERAL("file_3")); + ASSERT_FALSE(FileExists(file_path_3)); + + file_handle.reset(LoggingOpenFileForWrite(file_path_3, + FileWriteMode::kReuseOrCreate, + FilePermissions::kWorldReadable)); + EXPECT_NE(kInvalidFileHandle, file_handle); + EXPECT_TRUE(FileExists(file_path_3)); + EXPECT_EQ(0, FileSize(file_path_3)); +} + enum class ReadOrWrite : bool { kRead, kWrite, diff --git a/util/file/file_io_win.cc b/util/file/file_io_win.cc index 24a87501..dce1518a 100644 --- a/util/file/file_io_win.cc +++ b/util/file/file_io_win.cc @@ -43,6 +43,9 @@ FileHandle LoggingOpenFileForOutput(DWORD access, FilePermissions permissions) { DWORD disposition = 0; switch (mode) { + case FileWriteMode::kReuseOrFail: + disposition = OPEN_EXISTING; + break; case FileWriteMode::kReuseOrCreate: disposition = OPEN_ALWAYS; break;