diff options
author | fukino@chromium.org <fukino@chromium.org@0039d316-1c4b-4281-b951-d872f2087c98> | 2014-04-18 10:34:15 +0000 |
---|---|---|
committer | fukino@chromium.org <fukino@chromium.org@0039d316-1c4b-4281-b951-d872f2087c98> | 2014-04-18 10:34:15 +0000 |
commit | d8b565d03c31de38052f98c64c73bdf8f55afe6a (patch) | |
tree | 7ced567e14ed8820ddc227f085adb10d529f67ac | |
parent | 1aaee64c6b337fbbfa3ca495b975a84db26fdeb7 (diff) | |
download | chromium_src-d8b565d03c31de38052f98c64c73bdf8f55afe6a.zip chromium_src-d8b565d03c31de38052f98c64c73bdf8f55afe6a.tar.gz chromium_src-d8b565d03c31de38052f98c64c73bdf8f55afe6a.tar.bz2 |
base::ReadFile() should return the number of read bytes on Windows.
base::ReadFile() has inconsistent behavior between Linux/Mac and Windows.
When the passed |size| argument is larger than file's size, ReadFile() on Linux/Mac returns the number of read bytes, but ReadFile() on Windows returns -1.
We can workaround this behavior by calling GetFileSize() first, but there are already many clients which treat |size| as max size (They're passing the size of buffer).
I think we should make behavior on Windows be consistent with Linux/Mac.
BUG=243885
TEST=run base_unittests
Review URL: https://codereview.chromium.org/240893002
git-svn-id: svn://svn.chromium.org/chrome/trunk/src@264768 0039d316-1c4b-4281-b951-d872f2087c98
-rw-r--r-- | base/file_util.h | 6 | ||||
-rw-r--r-- | base/file_util_posix.cc | 16 | ||||
-rw-r--r-- | base/file_util_unittest.cc | 46 | ||||
-rw-r--r-- | base/file_util_win.cc | 6 |
4 files changed, 60 insertions, 14 deletions
diff --git a/base/file_util.h b/base/file_util.h index 487a171..7ca2d38 100644 --- a/base/file_util.h +++ b/base/file_util.h @@ -309,9 +309,9 @@ BASE_EXPORT bool CloseFile(FILE* file); // This is a cross-platform analog to Windows' SetEndOfFile() function. BASE_EXPORT bool TruncateFile(FILE* file); -// Reads the given number of bytes from the file into the buffer. Returns -// the number of read bytes, or -1 on error. -BASE_EXPORT int ReadFile(const FilePath& filename, char* data, int size); +// Reads at most the given number of bytes from the file into the buffer. +// Returns the number of read bytes, or -1 on error. +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. diff --git a/base/file_util_posix.cc b/base/file_util_posix.cc index e2eb2f8..f04053d 100644 --- a/base/file_util_posix.cc +++ b/base/file_util_posix.cc @@ -651,15 +651,15 @@ FILE* OpenFile(const FilePath& filename, const char* mode) { return result; } -int ReadFile(const FilePath& filename, char* data, int size) { +int ReadFile(const FilePath& filename, char* data, int max_size) { ThreadRestrictions::AssertIOAllowed(); int fd = HANDLE_EINTR(open(filename.value().c_str(), O_RDONLY)); if (fd < 0) return -1; - ssize_t bytes_read = HANDLE_EINTR(read(fd, data, size)); - if (int ret = IGNORE_EINTR(close(fd)) < 0) - return ret; + ssize_t bytes_read = HANDLE_EINTR(read(fd, data, max_size)); + if (IGNORE_EINTR(close(fd)) < 0) + return -1; return bytes_read; } @@ -670,8 +670,8 @@ int WriteFile(const FilePath& filename, const char* data, int size) { return -1; int bytes_written = WriteFileDescriptor(fd, data, size); - if (int ret = IGNORE_EINTR(close(fd)) < 0) - return ret; + if (IGNORE_EINTR(close(fd)) < 0) + return -1; return bytes_written; } @@ -697,8 +697,8 @@ int AppendToFile(const FilePath& filename, const char* data, int size) { return -1; int bytes_written = WriteFileDescriptor(fd, data, size); - if (int ret = IGNORE_EINTR(close(fd)) < 0) - return ret; + if (IGNORE_EINTR(close(fd)) < 0) + return -1; return bytes_written; } diff --git a/base/file_util_unittest.cc b/base/file_util_unittest.cc index 642c9d5..0d79391 100644 --- a/base/file_util_unittest.cc +++ b/base/file_util_unittest.cc @@ -21,6 +21,7 @@ #include <algorithm> #include <fstream> #include <set> +#include <vector> #include "base/base_paths.h" #include "base/file_util.h" @@ -1960,6 +1961,51 @@ TEST_F(FileUtilTest, AppendToFile) { EXPECT_EQ(L"hellohello", read_content); } +TEST_F(FileUtilTest, ReadFile) { + // Create a test file to be read. + const std::string kTestData("The quick brown fox jumps over the lazy dog."); + FilePath file_path = + temp_dir_.path().Append(FILE_PATH_LITERAL("ReadFileTest")); + + ASSERT_EQ(static_cast<int>(kTestData.size()), + WriteFile(file_path, kTestData.data(), kTestData.size())); + + // Make buffers with various size. + std::vector<char> small_buffer(kTestData.size() / 2); + std::vector<char> exact_buffer(kTestData.size()); + std::vector<char> large_buffer(kTestData.size() * 2); + + // Read the file with smaller buffer. + int bytes_read_small = ReadFile( + file_path, &small_buffer[0], static_cast<int>(small_buffer.size())); + EXPECT_EQ(bytes_read_small, static_cast<int>(small_buffer.size())); + EXPECT_EQ( + std::string(small_buffer.begin(), small_buffer.end()), + std::string(kTestData.begin(), kTestData.begin() + small_buffer.size())); + + // Read the file with buffer which have exactly same size. + int bytes_read_exact = ReadFile( + file_path, &exact_buffer[0], static_cast<int>(exact_buffer.size())); + EXPECT_EQ(bytes_read_exact, static_cast<int>(kTestData.size())); + EXPECT_EQ(std::string(exact_buffer.begin(), exact_buffer.end()), kTestData); + + // Read the file with larger buffer. + int bytes_read_large = ReadFile( + file_path, &large_buffer[0], static_cast<int>(large_buffer.size())); + EXPECT_EQ(bytes_read_large, static_cast<int>(kTestData.size())); + EXPECT_EQ(std::string(large_buffer.begin(), + large_buffer.begin() + kTestData.size()), + kTestData); + + // Make sure the return value is -1 if the file doesn't exist. + FilePath file_path_not_exist = + temp_dir_.path().Append(FILE_PATH_LITERAL("ReadFileNotExistTest")); + EXPECT_EQ(-1, + ReadFile(file_path_not_exist, + &exact_buffer[0], + static_cast<int>(exact_buffer.size()))); +} + TEST_F(FileUtilTest, ReadFileToString) { const char kTestData[] = "0123"; std::string data; diff --git a/base/file_util_win.cc b/base/file_util_win.cc index 94b545a..eb9c6dc 100644 --- a/base/file_util_win.cc +++ b/base/file_util_win.cc @@ -571,7 +571,7 @@ FILE* OpenFile(const FilePath& filename, const char* mode) { return _wfsopen(filename.value().c_str(), w_mode.c_str(), _SH_DENYNO); } -int ReadFile(const FilePath& filename, char* data, int size) { +int ReadFile(const FilePath& filename, char* data, int max_size) { ThreadRestrictions::AssertIOAllowed(); base::win::ScopedHandle file(CreateFile(filename.value().c_str(), GENERIC_READ, @@ -584,9 +584,9 @@ int ReadFile(const FilePath& filename, char* data, int size) { return -1; DWORD read; - if (::ReadFile(file, data, size, &read, NULL) && - static_cast<int>(read) == size) + if (::ReadFile(file, data, max_size, &read, NULL)) return read; + return -1; } |