Do not use regex to parse /proc/config.gz.
During boot, a compatibility check is performed and
/proc/config.gz is parsed. /proc/config.gz has
a well defined format:
# CONFIG_FOO is not set
CONFIG_BAR=y
with no trailing / leading spaces, spaces around
'=', or trailing comments. Using regex is slow
and unnecessary, hence a normal string search
is performed instead.
On the other hand, for assemble_vintf that needs to parse
android-base.cfg, because the file is maintained by
humans, free format is accepted, and regex is used.
Test: boots (boot time compat check passes)
Test: boot time regression no longer happens (~120ms vs. ~90ms)
Test: libvintf_test
Bug: 63537988
Change-Id: If6ab207d40459689386aaf40cd835effc5597886
diff --git a/KernelConfigParser.cpp b/KernelConfigParser.cpp
index 44cbf07..a0955e2 100644
--- a/KernelConfigParser.cpp
+++ b/KernelConfigParser.cpp
@@ -28,7 +28,8 @@
namespace android {
namespace vintf {
-KernelConfigParser::KernelConfigParser(bool processComments) : mProcessComments(processComments) {}
+KernelConfigParser::KernelConfigParser(bool processComments, bool relaxedFormat)
+ : mProcessComments(processComments), mRelaxedFormat(relaxedFormat) {}
status_t KernelConfigParser::finish() {
return process("\n", 1 /* sizeof "\n" */);
@@ -61,12 +62,28 @@
}
std::smatch match;
- if (std::regex_match(mRemaining, match, sKeyValuePattern)) {
- if (mConfigs.emplace(match[1], trimTrailingSpaces(match[2])).second) {
- return OK;
+
+ if (mRelaxedFormat) {
+ // Allow free format like " CONFIG_FOO = bar #trailing comments"
+ if (std::regex_match(mRemaining, match, sKeyValuePattern)) {
+ if (mConfigs.emplace(match[1], trimTrailingSpaces(match[2])).second) {
+ return OK;
+ }
+ mError << "Duplicated key in configs: " << match[1] << "\n";
+ return UNKNOWN_ERROR;
}
- mError << "Duplicated key in configs: " << match[1] << "\n";
- return UNKNOWN_ERROR;
+ } else {
+ // No spaces. Strictly like "CONFIG_FOO=bar"
+ size_t equalPos = mRemaining.find('=');
+ if (equalPos != std::string::npos) {
+ std::string key = mRemaining.substr(0, equalPos);
+ std::string value = mRemaining.substr(equalPos + 1);
+ if (mConfigs.emplace(std::move(key), std::move(value)).second) {
+ return OK;
+ }
+ mError << "Duplicated key in configs: " << mRemaining.substr(0, equalPos) << "\n";
+ return UNKNOWN_ERROR;
+ }
}
if (mProcessComments && std::regex_match(mRemaining, match, sNotSetPattern)) {
@@ -78,8 +95,16 @@
return UNKNOWN_ERROR;
}
- if (std::regex_match(mRemaining, match, sCommentPattern)) {
- return OK;
+ if (mRelaxedFormat) {
+ // Allow free format like " #comments here"
+ if (std::regex_match(mRemaining, match, sCommentPattern)) {
+ return OK;
+ }
+ } else {
+ // No leading spaces before the comment
+ if (mRemaining.at(0) == '#') {
+ return OK;
+ }
}
mError << "Unrecognized line in configs: " << mRemaining << "\n";
diff --git a/assemble_vintf.cpp b/assemble_vintf.cpp
index 7683472..2752078 100644
--- a/assemble_vintf.cpp
+++ b/assemble_vintf.cpp
@@ -65,7 +65,7 @@
std::cerr << "File '" << path << "' does not exist or cannot be read." << std::endl;
return false;
}
- KernelConfigParser parser(true /* processComments */);
+ KernelConfigParser parser(true /* processComments */, true /* relaxedFormat */);
std::string content = read(ifs);
status_t err = parser.process(content.c_str(), content.size());
if (err != OK) {
diff --git a/include/vintf/KernelConfigParser.h b/include/vintf/KernelConfigParser.h
index bad66a5..06a034d 100644
--- a/include/vintf/KernelConfigParser.h
+++ b/include/vintf/KernelConfigParser.h
@@ -29,7 +29,7 @@
class KernelConfigParser {
public:
- KernelConfigParser(bool processComments = false);
+ KernelConfigParser(bool processComments = false, bool relaxedFormat = false);
status_t process(const char* buf, size_t len);
status_t finish();
@@ -43,6 +43,7 @@
std::stringstream mError;
std::string mRemaining;
bool mProcessComments;
+ bool mRelaxedFormat;
};
} // namespace vintf
diff --git a/test/main.cpp b/test/main.cpp
index b3b8a59..8dc5ddb 100644
--- a/test/main.cpp
+++ b/test/main.cpp
@@ -1310,8 +1310,9 @@
EXPECT_EQ(matrix.getXmlSchemaPath("media_profile", {2, 0}), "");
}
-std::pair<KernelConfigParser, status_t> processData(const std::string& data, bool processComments) {
- KernelConfigParser parser(processComments);
+std::pair<KernelConfigParser, status_t> processData(const std::string& data, bool processComments,
+ bool relaxedFormat = false) {
+ KernelConfigParser parser(processComments, relaxedFormat);
const char* p = data.c_str();
size_t n = 0;
size_t chunkSize;
@@ -1378,7 +1379,7 @@
"CONFIG_WORLD=hello world! \n"
"CONFIG_GOOD = good morning! #comments here\n"
" CONFIG_MORNING = good morning! \n";
- auto pair = processData(data, true /* processComments */);
+ auto pair = processData(data, true /* processComments */, true /* relaxedFormat */);
ASSERT_EQ(OK, pair.second) << pair.first.error();
const auto& configs = pair.first.configs();
@@ -1403,7 +1404,7 @@
TEST_P(KernelConfigParserInvalidTest, NonSet1) {
const std::string data = "# CONFIG_NOT_EXIST is not sat\n";
- auto pair = processData(data, GetParam() /* processComments */);
+ auto pair = processData(data, GetParam() /* processComments */, true /* relaxedFormat */);
ASSERT_EQ(OK, pair.second) << pair.first.error();
const auto& configs = pair.first.configs();
EXPECT_EQ(configs.find("CONFIG_NOT_EXIST"), configs.end())
@@ -1412,12 +1413,14 @@
TEST_P(KernelConfigParserInvalidTest, InvalidLine1) {
const std::string data = "FOO_CONFIG=foo\n";
- ASSERT_NE(OK, processData(data, GetParam() /* processComments */).second);
+ ASSERT_NE(OK,
+ processData(data, GetParam() /* processComments */, true /* relaxedFormat */).second);
}
TEST_P(KernelConfigParserInvalidTest, InvalidLine2) {
const std::string data = "CONFIG_BAR-BAZ=foo\n";
- ASSERT_NE(OK, processData(data, GetParam() /* processComments */).second);
+ ASSERT_NE(OK,
+ processData(data, GetParam() /* processComments */, true /* relaxedFormat */).second);
}
INSTANTIATE_TEST_CASE_P(KernelConfigParser, KernelConfigParserInvalidTest, ::testing::Bool());