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
CrOS-Libchrome-Original-Commit: d8b565d03c31de38052f98c64c73bdf8f55afe6a
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 @@
// 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 @@
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 @@
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 @@
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 @@
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;