Parser::ParseConfigFile returns Result<void>
ParseApexConfigs() uses Parser::ParseConfigFile() to parse .rc files in
the target apex. ParseConfigFile() returning bool (with logging on
error) doesn't propagate the error message back to the callers
(including apexd or PackageManager).
We'd better to migrate other Parse*() methods of Parser class to return
Result<T>. But this change focuses on plumbing error progagation for
APEX configs.
Bug: 238820991
Test: atest CtsInitTestCases
Change-Id: Ifad97635dbb53a70053ec73a7a5b7e742466daf6
diff --git a/init/apex_init_util.cpp b/init/apex_init_util.cpp
index d618a6e..c818f8f 100644
--- a/init/apex_init_util.cpp
+++ b/init/apex_init_util.cpp
@@ -18,7 +18,6 @@
#include <glob.h>
-#include <map>
#include <vector>
#include <android-base/logging.h>
@@ -66,18 +65,20 @@
}
static Result<void> ParseConfigs(const std::vector<std::string>& configs) {
- Parser parser = CreateApexConfigParser(ActionManager::GetInstance(),
- ServiceList::GetInstance());
- bool success = true;
+ Parser parser =
+ CreateApexConfigParser(ActionManager::GetInstance(), ServiceList::GetInstance());
+ std::vector<std::string> errors;
for (const auto& c : configs) {
- success &= parser.ParseConfigFile(c);
+ auto result = parser.ParseConfigFile(c);
+ // We should handle other config files even when there's an error.
+ if (!result.ok()) {
+ errors.push_back(result.error().message());
+ }
}
-
- if (success) {
- return {};
- } else {
- return Error() << "Unable to parse apex configs";
+ if (!errors.empty()) {
+ return Error() << "Unable to parse apex configs: " << base::Join(errors, "|");
}
+ return {};
}
Result<void> ParseApexConfigs(const std::string& apex_name) {
diff --git a/init/parser.cpp b/init/parser.cpp
index 0a388db..adb41ad 100644
--- a/init/parser.cpp
+++ b/init/parser.cpp
@@ -141,19 +141,19 @@
return true;
}
-bool Parser::ParseConfigFile(const std::string& path) {
+Result<void> Parser::ParseConfigFile(const std::string& path) {
LOG(INFO) << "Parsing file " << path << "...";
android::base::Timer t;
auto config_contents = ReadFile(path);
if (!config_contents.ok()) {
- LOG(INFO) << "Unable to read config file '" << path << "': " << config_contents.error();
- return false;
+ return Error() << "Unable to read config file '" << path
+ << "': " << config_contents.error();
}
ParseData(path, &config_contents.value());
LOG(VERBOSE) << "(Parsing " << path << " took " << t << ".)";
- return true;
+ return {};
}
bool Parser::ParseConfigDir(const std::string& path) {
@@ -176,8 +176,8 @@
// Sort first so we load files in a consistent order (bug 31996208)
std::sort(files.begin(), files.end());
for (const auto& file : files) {
- if (!ParseConfigFile(file)) {
- LOG(ERROR) << "could not import file '" << file << "'";
+ if (auto result = ParseConfigFile(file); !result.ok()) {
+ LOG(ERROR) << "could not import file '" << file << "': " << result.error();
}
}
return true;
@@ -187,7 +187,11 @@
if (is_dir(path.c_str())) {
return ParseConfigDir(path);
}
- return ParseConfigFile(path);
+ auto result = ParseConfigFile(path);
+ if (!result.ok()) {
+ LOG(INFO) << result.error();
+ }
+ return result.ok();
}
} // namespace init
diff --git a/init/parser.h b/init/parser.h
index 95b0cd7..980ae0c 100644
--- a/init/parser.h
+++ b/init/parser.h
@@ -72,7 +72,7 @@
Parser();
bool ParseConfig(const std::string& path);
- bool ParseConfigFile(const std::string& path);
+ Result<void> ParseConfigFile(const std::string& path);
void AddSectionParser(const std::string& name, std::unique_ptr<SectionParser> parser);
void AddSingleLineParser(const std::string& prefix, LineCallback callback);