From 9bb7971ae794ecfc6dcaab28d13d3ade8ab18517 Mon Sep 17 00:00:00 2001 From: Elliott Hughes Date: Mon, 20 Mar 2017 19:16:18 -0700 Subject: [PATCH] Keep the ReadFileToString/ReadFdToString overhead down. Bug: https://code.google.com/p/android/issues/detail?id=258500 Bug: http://b/36046324 Test: ran tests Change-Id: I40e76a6dd164ea9a5e8e18159f543e1bb221dcba --- base/file.cpp | 8 ++++++++ base/file_test.cpp | 43 +++++++++++++++++++++++++++++++++++++++++++ 2 files changed, 51 insertions(+) diff --git a/base/file.cpp b/base/file.cpp index 378a405bc..d4e58942c 100644 --- a/base/file.cpp +++ b/base/file.cpp @@ -49,6 +49,14 @@ using namespace android::base::utf8; bool ReadFdToString(int fd, std::string* content) { content->clear(); + // Although original we had small files in mind, this code gets used for + // very large files too, where the std::string growth heuristics might not + // be suitable. https://code.google.com/p/android/issues/detail?id=258500. + struct stat sb; + if (fstat(fd, &sb) != -1 && sb.st_size > 0) { + content->reserve(sb.st_size); + } + char buf[BUFSIZ]; ssize_t n; while ((n = TEMP_FAILURE_RETRY(read(fd, &buf[0], sizeof(buf)))) > 0) { diff --git a/base/file_test.cpp b/base/file_test.cpp index 266131ecf..02b431de7 100644 --- a/base/file_test.cpp +++ b/base/file_test.cpp @@ -214,3 +214,46 @@ TEST(file, Dirname) { EXPECT_EQ(".", android::base::Dirname("sh")); EXPECT_EQ("/system/bin", android::base::Dirname("/system/bin/sh/")); } + +TEST(file, ReadFileToString_capacity) { + TemporaryFile tf; + ASSERT_TRUE(tf.fd != -1); + + // For a huge file, the overhead should still be small. + std::string s; + size_t size = 16 * 1024 * 1024; + ASSERT_TRUE(android::base::WriteStringToFile(std::string(size, 'x'), tf.path)); + ASSERT_TRUE(android::base::ReadFileToString(tf.path, &s)); + EXPECT_EQ(size, s.size()); + EXPECT_LT(s.capacity(), size + 16); + + // Even for weird badly-aligned sizes. + size += 12345; + ASSERT_TRUE(android::base::WriteStringToFile(std::string(size, 'x'), tf.path)); + ASSERT_TRUE(android::base::ReadFileToString(tf.path, &s)); + EXPECT_EQ(size, s.size()); + EXPECT_LT(s.capacity(), size + 16); + + // We'll shrink an enormous string if you read a small file into it. + size = 64; + ASSERT_TRUE(android::base::WriteStringToFile(std::string(size, 'x'), tf.path)); + ASSERT_TRUE(android::base::ReadFileToString(tf.path, &s)); + EXPECT_EQ(size, s.size()); + EXPECT_LT(s.capacity(), size + 16); +} + +TEST(file, ReadFileToString_capacity_0) { + TemporaryFile tf; + ASSERT_TRUE(tf.fd != -1); + + // Because /proc reports its files as zero-length, we don't actually trust + // any file that claims to be zero-length. Rather than add increasingly + // complex heuristics for shrinking the passed-in string in that case, we + // currently leave it alone. + std::string s; + size_t initial_capacity = s.capacity(); + ASSERT_TRUE(android::base::WriteStringToFile("", tf.path)); + ASSERT_TRUE(android::base::ReadFileToString(tf.path, &s)); + EXPECT_EQ(0U, s.size()); + EXPECT_EQ(initial_capacity, s.capacity()); +}