inode2filename: Fix a bug on parsing lines for inode2filename.
Some temp file has '\n' in its name, which breaks the parsing.
To solve this issue, add a rest line length in the begining as a 4 bytes
array. For example, suppose the line is "0x000x000x10K 253:9:7 ./test",
"0x000x000x0000x10" (16) is the length of "K 253:9:7 ./test".
Each line has no '\n' in the end and the end is told by the length.
Bug: 158488826
Test: ioraiorap.inode2filename -pm out --all -of textcache -v
Test: atest iorapd-tests
Change-Id: Id950e7e0ba3e5f76b336ba95da79d0279b0bf472
(cherry picked from commit 76f078f576a7dd29c58ba91ca021f5409433a42d)
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 @@
<< 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 @@
<< 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 @@
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 @@
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 @@
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 @@
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
