summaryrefslogtreecommitdiffstats
diff options
context:
space:
mode:
authorfukino@chromium.org <fukino@chromium.org@0039d316-1c4b-4281-b951-d872f2087c98>2014-04-18 10:34:15 +0000
committerfukino@chromium.org <fukino@chromium.org@0039d316-1c4b-4281-b951-d872f2087c98>2014-04-18 10:34:15 +0000
commitd8b565d03c31de38052f98c64c73bdf8f55afe6a (patch)
tree7ced567e14ed8820ddc227f085adb10d529f67ac
parent1aaee64c6b337fbbfa3ca495b975a84db26fdeb7 (diff)
downloadchromium_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.h6
-rw-r--r--base/file_util_posix.cc16
-rw-r--r--base/file_util_unittest.cc46
-rw-r--r--base/file_util_win.cc6
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;
}