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

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<std::vector<std::string>> CollectApexConfigs(const std::string& ap
 }
 
 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) {

init/parser.cpp

@@ -141,19 +141,19 @@ bool Parser::ParseConfigFileInsecure(const std::string& path) {
     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 @@ bool Parser::ParseConfigDir(const std::string& path) {
     // 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 @@ bool Parser::ParseConfig(const std::string& path) {
     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

init/parser.h

@@ -72,7 +72,7 @@ class Parser {
     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);