Merge ab/7061308 into stage.temp_RQ2A.210305.007

Bug: 180401296
Merged-In: I06aa72fc38ffabc20623dfe531a19c268d936b07
Change-Id: I05335737d1a64c8bd133025d91d3d714703512b3

4 files changed, 130 insertions, 62 deletions

diff --git a/src/inode2filename/main.cc b/src/inode2filename/main.cc
index 38d6eab..986f6ac 100644
--- a/src/inode2filename/main.cc
+++ b/src/inode2filename/main.cc
@@ -20,6 +20,7 @@
 
 #include <iostream>
 #include <fstream>
+#include <sstream>
 #include <string_view>
 
 #if defined(IORAP_INODE2FILENAME_MAIN)
@@ -384,9 +385,16 @@ int main(int argc, char** argv) {
              << result.inode
              << " \"" << result.data.value() << "\"" << std::endl;
       } else if (output_format == OutputFormatKind::kIpc) {
-        fout << "K "
-             << result.inode
-             << " " << result.data.value() << std::endl;
+        std::stringstream stream;
+        stream << "K " << result.inode << " " << result.data.value();
+        std::string line = stream.str();
+
+        // Convert the size to 4 bytes.
+        int32_t size = line.size();
+        char buf[sizeof(int32_t)];
+        memcpy(buf, &size, sizeof(int32_t));
+        fout.write(buf, sizeof(int32_t));
+        fout.write(line.c_str(), size);
       } else if (output_format == OutputFormatKind::kTextCache) {
         // Same format as TextCacheDataSource (system/extras/pagecache/pagecache.py -d)
         //   "$device_number $inode $filesize $filename..."
@@ -407,9 +415,16 @@ int main(int argc, char** argv) {
              << result.inode
              << " '" << *result.ErrorMessage() << "'" << std::endl;
       } else if (output_format == OutputFormatKind::kIpc) {
-        fout << "E "
-             << result.inode
-             << " " << result.data.error() << std::endl;
+        std::stringstream stream;
+        stream << "E " << result.inode << " " << result.data.error() << std::endl;
+        std::string line = stream.str();
+
+        // Convert the size to 4 bytes.
+        int32_t size = line.size();
+        char buf[sizeof(int32_t)];
+        memcpy(buf, &size, sizeof(int32_t));
+        fout.write(buf, sizeof(int32_t));
+        fout.write(line.c_str(), size);
       }
       else if (output_format == OutputFormatKind::kTextCache) {
         // Don't add bad results to the textcache. They are dropped.
diff --git a/src/inode2filename/out_of_process_inode_resolver.cc b/src/inode2filename/out_of_process_inode_resolver.cc
index 88ce6e0..f409fd2 100644
--- a/src/inode2filename/out_of_process_inode_resolver.cc
+++ b/src/inode2filename/out_of_process_inode_resolver.cc
@@ -72,64 +72,41 @@ std::ios_base::failure IosBaseFailureWithErrno(const char* message) {
 
 static constexpr bool kDebugFgets = false;
 
-// This always contains the 'newline' character at the end of the string.
-// If there is not, the string is both empty and we hit EOF (or an error occurred).
-std::string FgetsWholeLine(FILE* stream,
-                           bool* eof) {
+int32_t ReadLineLength(FILE* stream, bool* eof) {
+  char buf[sizeof(int32_t)];
+  size_t count = fread(buf, 1, sizeof(int32_t), stream);
+  if (feof(stream)) {
+    // If reaching the end of the stream when trying to read the first int, just
+    // return. This is legitimate, because after reading the last line, the next
+    // iteration will reach this.
+    *eof = true;
+    return 0;
+  }
+  int32_t length;
+  memcpy(&length, buf, sizeof(int32_t));
+  return length;
+}
+
+// The steam is like [size1][file1][size2][file2]...[sizeN][fileN].
+std::string ReadOneLine(FILE* stream, bool* eof) {
   DCHECK(stream != nullptr);
   DCHECK(eof != nullptr);
 
-  char buf[1024];
-
-  std::string str;
-  *eof = false;
-
-  while (true) {
-    memset(buf, '\0', sizeof(buf));
-
-    char* out = fgets(&buf[0], sizeof(buf), stream);
-
-    if (out == nullptr) {
-      // either EOF or error.
-
-      *eof = true;
-      if (feof(stream)) {
-        return str;
-      } else {
-        // error! :(
-        PLOG(ERROR) << "failed to fgets";
-        return str;
-      }
-    }
-
-    if (kDebugFgets) {
-      std::string dbg;
-
-      for (size_t i = 0; i < sizeof(buf); ++i) {
-        if (buf[i] == '\0') {
-          break;
-        }
-
-        int val = buf[i];
-
-        dbg += "," + std::to_string(val);
-      }
-
-      LOG(DEBUG) << "fgets ascii: " << dbg;
-    }
-
-    str += buf;
-
-    // fgets always reads at most count-1 characters.
-    // the last character is always '\0'
-    // the second-to-last character would be \n if we read the full line,
-    // and any other character otherwise.
-    if (!str.empty() && str.back() == '\n') {
-      // we read the whole line: do not need to call fgets again.
-      break;
-    }
+  int32_t length = ReadLineLength(stream, eof);
+  if (length <= 0) {
+    PLOG(ERROR) << "unexpected 0 length line.";
+    *eof = true;
+    return "";
   }
 
+  std::string str(length, '\0');
+  size_t count = fread(&str[0], sizeof(char), length, stream);
+  if (feof(stream) || ferror(stream) || count != (uint32_t)length) {
+    // error! :(
+    PLOG(ERROR) << "unexpected end of the line during fread";
+    *eof = true;
+    return "";
+  }
   return str;
 }
 
@@ -192,10 +169,10 @@ std::optional<InodeResult> ParseFromLine(const std::string& line) {
     return InodeResult::makeFailure(inode, error_code);
   } else if (result_ok == true) {
     std::string rest_of_line;
+    ss >> rest_of_line;
 
-    // parse " string with potential spaces[\n]"
+    // parse " string with potential spaces"
     // into "string with potential spaces"
-    std::getline(/*inout*/ss, /*out*/rest_of_line);
     LeftTrim(/*inout*/rest_of_line);
 
     if (ss.fail()) {
@@ -345,7 +322,7 @@ struct OutOfProcessInodeResolver::Impl {
 
     bool file_eof = false;
     while (!file_eof) {
-      std::string inode2filename_line = FgetsWholeLine(file_reader.get(), /*out*/&file_eof);
+      std::string inode2filename_line = ReadOneLine(file_reader.get(), /*out*/&file_eof);
 
       if (inode2filename_line.empty()) {
         if (!file_eof) {
diff --git a/src/inode2filename/out_of_process_inode_resolver.h b/src/inode2filename/out_of_process_inode_resolver.h
index c9f4291..d41e93f 100644
--- a/src/inode2filename/out_of_process_inode_resolver.h
+++ b/src/inode2filename/out_of_process_inode_resolver.h
@@ -39,6 +39,15 @@ class OutOfProcessInodeResolver : public InodeResolver {
   Impl* impl_;
 };
 
+// Reads one line data from the stream.
+// Each line is in the format of "<4 bytes line length><state> <inode info> <file path>"
+// The <4 bytes line length> is the length rest data of "<state> <inode info> <file path>".
+// The return string is "<state> <inode info> <file path>".
+// For example: for "<size>K 253:9:6 ./test", the return value is
+// "K 253:9:6 ./test". The <size> is encoded in the first 4 bytes.
+// Note: there is no newline in the end of each line and the line shouldn't be
+// empty unless there is some error.
+std::string ReadOneLine(FILE* stream, bool* eof);
 }
 
 #endif  // IORAP_SRC_INODE2FILENAME_OUT_OF_PROCESS_INDOE_RESOLVER_H_
diff --git a/tests/src/inode2filename/out_of_process_inode_resolver_test.cc b/tests/src/inode2filename/out_of_process_inode_resolver_test.cc
new file mode 100644
index 0000000..34ea532
--- /dev/null
+++ b/tests/src/inode2filename/out_of_process_inode_resolver_test.cc
@@ -0,0 +1,67 @@
+/*
+ * Copyright (C) 2020 The Android Open Source Project
+ *
+ * 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 "inode2filename/out_of_process_inode_resolver.h"
+
+#include <cstdio>
+#include <gmock/gmock.h>
+#include <gtest/gtest.h>
+#include <filesystem>
+#include <fstream>
+#include <iostream>
+#include <string>
+#include <vector>
+
+using ::testing::ElementsAre;
+
+namespace iorap::inode2filename {
+
+void WriteInt(int val, std::FILE* f) {
+  char buf[4];
+  memcpy(buf, &val, 4);
+  fwrite(buf, 1, 4, f);
+}
+
+TEST(OutOfProcessInodeResolverTest, ReadOneline) {
+  std::FILE* tmpf = std::tmpfile();
+
+  WriteInt(16, tmpf);
+  std::fputs("K 253:9:6 ./test", tmpf);
+  WriteInt(22, tmpf);
+  std::fputs("K 253:9:7 ./test\ntest\n", tmpf);
+  WriteInt(21, tmpf);
+  std::fputs("E 253:9:7 ./test\ntest", tmpf);
+  WriteInt(15, tmpf);
+  std::fputs("K 253:9:8 ./tmp", tmpf);
+
+  std::rewind(tmpf);
+  bool file_eof = false;
+  std::vector<std::string> result;
+
+  while (!file_eof) {
+    std::string line = ReadOneLine(tmpf, /*out*/&file_eof);
+    if (!line.empty()) {
+      result.push_back(line);
+    }
+  }
+
+  ASSERT_THAT(result, ElementsAre("K 253:9:6 ./test",
+                                  "K 253:9:7 ./test\ntest\n",
+                                  "E 253:9:7 ./test\ntest",
+                                  "K 253:9:8 ./tmp" ));
+}
+
+} // namespace iorap::inode2filename