From c188972e09d81c36c5a2946dc015adec36819583 Mon Sep 17 00:00:00 2001 From: Yifan Hong Date: Fri, 7 Jul 2017 16:19:52 -0700 Subject: Allow KernelConfigParser to be less restrictive Rewrite KernelConfigParser::processRemaining() to allow the following text: # CONFIG_NOT_SET is not set CONFIG_ONE=1 # 'tis a one! CONFIG_TWO=2 #'tis a two! CONFIG_THREE=3#'tis a three! CONFIG_233=233#'tis a three! CONFIG_Y=y CONFIG_YES=y#YES! CONFIG_STR=string CONFIG_HELLO=hello world! #still works CONFIG_WORLD=hello world! CONFIG_GOOD = good morning! #comments here CONFIG_MORNING = good morning! Test: libvintf_test Bug: 38324908 Change-Id: I1e8112b8cbf1f1d8a2b6b34daa9a593eff69cff2 --- KernelConfigParser.cpp | 54 ++++++++++++++++++++++++++------------------- test/main.cpp | 60 ++++++++++++++++++++++++++++++++++++++++++++++++++ 2 files changed, 91 insertions(+), 23 deletions(-) diff --git a/KernelConfigParser.cpp b/KernelConfigParser.cpp index 1d08fca..44cbf07 100644 --- a/KernelConfigParser.cpp +++ b/KernelConfigParser.cpp @@ -18,6 +18,13 @@ #include +#define KEY "(CONFIG[\\w_]+)" +#define COMMENT "(?:#.*)" + +static const std::regex sKeyValuePattern("^\\s*" KEY "\\s*=\\s*([^#]+)" COMMENT "?$"); +static const std::regex sNotSetPattern("^\\s*#\\s*" KEY " is not set\\s*$"); +static const std::regex sCommentPattern("^\\s*" COMMENT "$"); + namespace android { namespace vintf { @@ -39,43 +46,44 @@ const std::map& KernelConfigParser::configs() const { return mConfigs; } +// trim spaces between value and #, value and end of line +std::string trimTrailingSpaces(const std::string& s) { + auto r = s.rbegin(); + for (; r != s.rend() && std::isspace(*r); ++r) + ; + return std::string{s.begin(), r.base()}; +} + status_t KernelConfigParser::processRemaining() { - static std::regex sCommentPattern("^# (CONFIG[\\w_]+) is not set$"); if (mRemaining.empty()) { return OK; } - if (mRemaining[0] == '#') { - if (!mProcessComments) { + std::smatch match; + if (std::regex_match(mRemaining, match, sKeyValuePattern)) { + if (mConfigs.emplace(match[1], trimTrailingSpaces(match[2])).second) { return OK; } - std::smatch sm; - if (!std::regex_match(mRemaining, sm, sCommentPattern)) { - return OK; // ignore this comment; - } - if (!mConfigs.emplace(sm[1], "n").second) { - mError << "Key " << sm[1] << " is set but commented as not set" - << "\n"; - return UNKNOWN_ERROR; - } - - return OK; + mError << "Duplicated key in configs: " << match[1] << "\n"; + return UNKNOWN_ERROR; } - size_t equalPos = mRemaining.find('='); - if (equalPos == std::string::npos) { - mError << "Unrecognized line in configs: " << mRemaining << "\n"; + if (mProcessComments && std::regex_match(mRemaining, match, sNotSetPattern)) { + if (mConfigs.emplace(match[1], "n").second) { + return OK; + } + mError << "Key " << match[1] << " is set but commented as not set" + << "\n"; return UNKNOWN_ERROR; } - std::string key = mRemaining.substr(0, equalPos); - std::string value = mRemaining.substr(equalPos + 1); - if (!mConfigs.emplace(std::move(key), std::move(value)).second) { - mError << "Duplicated key in configs: " << mRemaining.substr(0, equalPos) << "\n"; - return UNKNOWN_ERROR; + + if (std::regex_match(mRemaining, match, sCommentPattern)) { + return OK; } - return OK; + mError << "Unrecognized line in configs: " << mRemaining << "\n"; + return UNKNOWN_ERROR; } status_t KernelConfigParser::process(const char* buf, size_t len) { diff --git a/test/main.cpp b/test/main.cpp index 179407f..b3b8a59 100644 --- a/test/main.cpp +++ b/test/main.cpp @@ -1362,6 +1362,66 @@ TEST_F(LibVintfTest, KernelConfigParser2) { EXPECT_EQ(configs.find("CONFIG_NOT_SET2")->second, "n"); } +TEST_F(LibVintfTest, KernelConfigParserSpace) { + // usage in android-base.cfg + const std::string data = + " # CONFIG_NOT_SET is not set \n" + " CONFIG_ONE=1 # 'tis a one!\n" + " CONFIG_TWO=2 #'tis a two! \n" + " CONFIG_THREE=3#'tis a three! \n" + " CONFIG_233=233#'tis a three! \n" + "#yey! random comments\n" + "CONFIG_Y=y \n" + " CONFIG_YES=y#YES! \n" + "CONFIG_STR=string\n" + "CONFIG_HELLO=hello world! #still works\n" + "CONFIG_WORLD=hello world! \n" + "CONFIG_GOOD = good morning! #comments here\n" + " CONFIG_MORNING = good morning! \n"; + auto pair = processData(data, true /* processComments */); + ASSERT_EQ(OK, pair.second) << pair.first.error(); + const auto& configs = pair.first.configs(); + + EXPECT_EQ(configs.find("CONFIG_ONE")->second, "1"); + EXPECT_EQ(configs.find("CONFIG_TWO")->second, "2"); + EXPECT_EQ(configs.find("CONFIG_THREE")->second, "3"); + EXPECT_EQ(configs.find("CONFIG_Y")->second, "y"); + EXPECT_EQ(configs.find("CONFIG_STR")->second, "string"); + EXPECT_EQ(configs.find("CONFIG_HELLO")->second, "hello world!") + << "Value should be \"hello world!\" without trailing spaces"; + EXPECT_EQ(configs.find("CONFIG_WORLD")->second, "hello world!") + << "Value should be \"hello world!\" without trailing spaces"; + EXPECT_EQ(configs.find("CONFIG_GOOD")->second, "good morning!") + << "Value should be \"good morning!\" without leading or trailing spaces"; + EXPECT_EQ(configs.find("CONFIG_MORNING")->second, "good morning!") + << "Value should be \"good morning!\" without leading or trailing spaces"; + EXPECT_EQ(configs.find("CONFIG_NOT_SET")->second, "n"); +} + +// Run KernelConfigParserInvalidTest on processComments = {true, false} +class KernelConfigParserInvalidTest : public ::testing::TestWithParam {}; + +TEST_P(KernelConfigParserInvalidTest, NonSet1) { + const std::string data = "# CONFIG_NOT_EXIST is not sat\n"; + auto pair = processData(data, GetParam() /* processComments */); + ASSERT_EQ(OK, pair.second) << pair.first.error(); + const auto& configs = pair.first.configs(); + EXPECT_EQ(configs.find("CONFIG_NOT_EXIST"), configs.end()) + << "CONFIG_NOT_EXIST should not exist because of typo"; +} + +TEST_P(KernelConfigParserInvalidTest, InvalidLine1) { + const std::string data = "FOO_CONFIG=foo\n"; + ASSERT_NE(OK, processData(data, GetParam() /* processComments */).second); +} + +TEST_P(KernelConfigParserInvalidTest, InvalidLine2) { + const std::string data = "CONFIG_BAR-BAZ=foo\n"; + ASSERT_NE(OK, processData(data, GetParam() /* processComments */).second); +} + +INSTANTIATE_TEST_CASE_P(KernelConfigParser, KernelConfigParserInvalidTest, ::testing::Bool()); + } // namespace vintf } // namespace android -- cgit v1.2.3-54-g00ecf