init: clean up the SectionParser interface and Parser class

Remove the dependency on Action and Service from what should be a
generic Parser class.

Make ActionParser, ImportParser, and ServiceParser take a pointer to
their associated classes instead of accessing them through a
singleton.

Misc fixes to SectionParser Interface:
1) Make SectionParser::ParseLineSection() non-const as it always should
have been.
2) Use Rvalue references where appropriate
3) Remove extra std::string& filename in SectionParser::EndFile()
4) Only have SectionParser::ParseSection() as pure virtual

Document SectionParser.

Make ImportParser report the filename and line number of failed imports.

Make ServiceParser report the filename and line number of duplicated services.

Test: Boot bullhead

Change-Id: I86568a5b375fb4f27f4cb235ed1e37635f01d630
diff --git a/init/action.cpp b/init/action.cpp
index c128968..8d49e2a 100644
--- a/init/action.cpp
+++ b/init/action.cpp
@@ -363,7 +363,7 @@
     }
 }
 
-bool ActionParser::ParseSection(const std::vector<std::string>& args, const std::string& filename,
+bool ActionParser::ParseSection(std::vector<std::string>&& args, const std::string& filename,
                                 int line, std::string* err) {
     std::vector<std::string> triggers(args.begin() + 1, args.end());
     if (triggers.size() < 1) {
@@ -380,13 +380,12 @@
     return true;
 }
 
-bool ActionParser::ParseLineSection(const std::vector<std::string>& args, int line,
-                                    std::string* err) {
-    return action_ ? action_->AddCommand(args, line, err) : false;
+bool ActionParser::ParseLineSection(std::vector<std::string>&& args, int line, std::string* err) {
+    return action_ ? action_->AddCommand(std::move(args), line, err) : false;
 }
 
 void ActionParser::EndSection() {
     if (action_ && action_->NumCommands() > 0) {
-        ActionManager::GetInstance().AddAction(std::move(action_));
+        action_manager_->AddAction(std::move(action_));
     }
 }
diff --git a/init/action.h b/init/action.h
index 25e5a3e..e099b58 100644
--- a/init/action.h
+++ b/init/action.h
@@ -114,16 +114,15 @@
 
 class ActionParser : public SectionParser {
   public:
-    ActionParser() : action_(nullptr) {
-    }
-    bool ParseSection(const std::vector<std::string>& args, const std::string& filename, int line,
+    ActionParser(ActionManager* action_manager)
+        : action_manager_(action_manager), action_(nullptr) {}
+    bool ParseSection(std::vector<std::string>&& args, const std::string& filename, int line,
                       std::string* err) override;
-    bool ParseLineSection(const std::vector<std::string>& args, int line, std::string* err) override;
+    bool ParseLineSection(std::vector<std::string>&& args, int line, std::string* err) override;
     void EndSection() override;
-    void EndFile(const std::string&) override {
-    }
 
   private:
+    ActionManager* action_manager_;
     std::unique_ptr<Action> action_;
 };
 
diff --git a/init/builtins.cpp b/init/builtins.cpp
index 43eb420..87d4e81 100644
--- a/init/builtins.cpp
+++ b/init/builtins.cpp
@@ -390,7 +390,7 @@
 
     // Turning this on and letting the INFO logging be discarded adds 0.2s to
     // Nexus 9 boot time, so it's disabled by default.
-    if (false) parser.DumpState();
+    if (false) DumpState();
 }
 
 /* mount_fstab
diff --git a/init/import_parser.cpp b/init/import_parser.cpp
index f66b2ba..99275e5 100644
--- a/init/import_parser.cpp
+++ b/init/import_parser.cpp
@@ -20,7 +20,7 @@
 
 #include "util.h"
 
-bool ImportParser::ParseSection(const std::vector<std::string>& args, const std::string& filename,
+bool ImportParser::ParseSection(std::vector<std::string>&& args, const std::string& filename,
                                 int line, std::string* err) {
     if (args.size() != 2) {
         *err = "single argument needed for import\n";
@@ -35,16 +35,18 @@
     }
 
     LOG(INFO) << "Added '" << conf_file << "' to import list";
-    imports_.emplace_back(std::move(conf_file));
+    if (filename_.empty()) filename_ = filename;
+    imports_.emplace_back(std::move(conf_file), line);
     return true;
 }
 
-void ImportParser::EndFile(const std::string& filename) {
+void ImportParser::EndFile() {
     auto current_imports = std::move(imports_);
     imports_.clear();
-    for (const auto& s : current_imports) {
-        if (!Parser::GetInstance().ParseConfig(s)) {
-            PLOG(ERROR) << "could not import file '" << s << "' from '" << filename << "'";
+    for (const auto& [import, line_num] : current_imports) {
+        if (!parser_->ParseConfig(import)) {
+            PLOG(ERROR) << filename_ << ": " << line_num << ": Could not import file '" << import
+                        << "'";
         }
     }
 }
diff --git a/init/import_parser.h b/init/import_parser.h
index e15d555..45cbfad 100644
--- a/init/import_parser.h
+++ b/init/import_parser.h
@@ -24,20 +24,17 @@
 
 class ImportParser : public SectionParser {
   public:
-    ImportParser()  {
-    }
-    bool ParseSection(const std::vector<std::string>& args, const std::string& filename, int line,
+    ImportParser(Parser* parser) : parser_(parser) {}
+    bool ParseSection(std::vector<std::string>&& args, const std::string& filename, int line,
                       std::string* err) override;
-    bool ParseLineSection(const std::vector<std::string>& args, int line,
-                          std::string* err) override {
-        return true;
-    }
-    void EndSection() override {
-    }
-    void EndFile(const std::string& filename) override;
+    void EndFile() override;
 
   private:
-    std::vector<std::string> imports_;
+    Parser* parser_;
+    // Store filename for later error reporting.
+    std::string filename_;
+    // Vector of imports and their line numbers for later error reporting.
+    std::vector<std::pair<std::string, int>> imports_;
 };
 
 #endif
diff --git a/init/init.cpp b/init/init.cpp
index 9a4aa24..33330e9 100644
--- a/init/init.cpp
+++ b/init/init.cpp
@@ -96,6 +96,11 @@
 static std::string wait_prop_name;
 static std::string wait_prop_value;
 
+void DumpState() {
+    ServiceManager::GetInstance().DumpState();
+    ActionManager::GetInstance().DumpState();
+}
+
 void register_epoll_handler(int fd, void (*fn)()) {
     epoll_event ev;
     ev.events = EPOLLIN;
@@ -1398,10 +1403,13 @@
     const BuiltinFunctionMap function_map;
     Action::set_function_map(&function_map);
 
+    ActionManager& am = ActionManager::GetInstance();
+    ServiceManager& sm = ServiceManager::GetInstance();
     Parser& parser = Parser::GetInstance();
-    parser.AddSectionParser("service",std::make_unique<ServiceParser>());
-    parser.AddSectionParser("on", std::make_unique<ActionParser>());
-    parser.AddSectionParser("import", std::make_unique<ImportParser>());
+
+    parser.AddSectionParser("service", std::make_unique<ServiceParser>(&sm));
+    parser.AddSectionParser("on", std::make_unique<ActionParser>(&am));
+    parser.AddSectionParser("import", std::make_unique<ImportParser>(&parser));
     std::string bootscript = GetProperty("ro.boot.init_rc", "");
     if (bootscript.empty()) {
         parser.ParseConfig("/init.rc");
@@ -1419,9 +1427,7 @@
 
     // Turning this on and letting the INFO logging be discarded adds 0.2s to
     // Nexus 9 boot time, so it's disabled by default.
-    if (false) parser.DumpState();
-
-    ActionManager& am = ActionManager::GetInstance();
+    if (false) DumpState();
 
     am.QueueEventTrigger("early-init");
 
@@ -1456,10 +1462,10 @@
         // By default, sleep until something happens.
         int epoll_timeout_ms = -1;
 
-        if (!(waiting_for_prop || ServiceManager::GetInstance().IsWaitingForExec())) {
+        if (!(waiting_for_prop || sm.IsWaitingForExec())) {
             am.ExecuteOneCommand();
         }
-        if (!(waiting_for_prop || ServiceManager::GetInstance().IsWaitingForExec())) {
+        if (!(waiting_for_prop || sm.IsWaitingForExec())) {
             restart_processes();
 
             // If there's a process that needs restarting, wake up in time for that.
diff --git a/init/init.h b/init/init.h
index 1da3350..6add75f 100644
--- a/init/init.h
+++ b/init/init.h
@@ -34,4 +34,6 @@
 
 bool start_waiting_for_property(const char *name, const char *value);
 
+void DumpState();
+
 #endif  /* _INIT_INIT_H */
diff --git a/init/init_parser.cpp b/init/init_parser.cpp
index b425497..5c7af79 100644
--- a/init/init_parser.cpp
+++ b/init/init_parser.cpp
@@ -17,14 +17,12 @@
 #include "init_parser.h"
 
 #include <dirent.h>
-#include <fcntl.h>
 
 #include <android-base/logging.h>
 #include <android-base/stringprintf.h>
 
-#include "action.h"
 #include "parser.h"
-#include "service.h"
+#include "util.h"
 
 Parser::Parser() {
 }
@@ -71,13 +69,14 @@
                 }
                 section_parser = section_parsers_[args[0]].get();
                 std::string ret_err;
-                if (!section_parser->ParseSection(args, state.filename, state.line, &ret_err)) {
+                if (!section_parser->ParseSection(std::move(args), state.filename, state.line,
+                                                  &ret_err)) {
                     parse_error(&state, "%s\n", ret_err.c_str());
                     section_parser = nullptr;
                 }
             } else if (section_parser) {
                 std::string ret_err;
-                if (!section_parser->ParseLineSection(args, state.line, &ret_err)) {
+                if (!section_parser->ParseLineSection(std::move(args), state.line, &ret_err)) {
                     parse_error(&state, "%s\n", ret_err.c_str());
                 }
             }
@@ -100,8 +99,8 @@
 
     data.push_back('\n'); // TODO: fix parse_config.
     ParseData(path, data);
-    for (const auto& sp : section_parsers_) {
-        sp.second->EndFile(path);
+    for (const auto& [section_name, section_parser] : section_parsers_) {
+        section_parser->EndFile();
     }
 
     LOG(VERBOSE) << "(Parsing " << path << " took " << t << ".)";
@@ -141,8 +140,3 @@
     }
     return ParseConfigFile(path);
 }
-
-void Parser::DumpState() const {
-    ServiceManager::GetInstance().DumpState();
-    ActionManager::GetInstance().DumpState();
-}
diff --git a/init/init_parser.h b/init/init_parser.h
index 64f0cac..acceed4 100644
--- a/init/init_parser.h
+++ b/init/init_parser.h
@@ -22,22 +22,42 @@
 #include <string>
 #include <vector>
 
+//  SectionParser is an interface that can parse a given 'section' in init.
+//
+//  You can implement up to 4 functions below, with ParseSection() being mandatory.
+//  The first two function return bool with false indicating a failure and has a std::string* err
+//  parameter into which an error string can be written.  It will be reported along with the
+//  filename and line number of where the error occurred.
+//
+//  1) bool ParseSection(std::vector<std::string>&& args, const std::string& filename,
+//                       int line, std::string* err)
+//    This function is called when a section is first encountered.
+//
+//  2) bool ParseLineSection(std::vector<std::string>&& args, int line, std::string* err)
+//    This function is called on each subsequent line until the next section is encountered.
+//
+//  3) bool EndSection()
+//    This function is called either when a new section is found or at the end of the file.
+//    It indicates that parsing of the current section is complete and any relevant objects should
+//    be committed.
+//
+//  4) bool EndFile()
+//    This function is called at the end of the file.
+//    It indicates that the parsing has completed and any relevant objects should be committed.
+
 class SectionParser {
-public:
-    virtual ~SectionParser() {
-    }
-    virtual bool ParseSection(const std::vector<std::string>& args, const std::string& filename,
+  public:
+    virtual ~SectionParser() {}
+    virtual bool ParseSection(std::vector<std::string>&& args, const std::string& filename,
                               int line, std::string* err) = 0;
-    virtual bool ParseLineSection(const std::vector<std::string>& args, int line,
-                                  std::string* err) = 0;
-    virtual void EndSection() = 0;
-    virtual void EndFile(const std::string& filename) = 0;
+    virtual bool ParseLineSection(std::vector<std::string>&&, int, std::string*) { return true; };
+    virtual void EndSection(){};
+    virtual void EndFile(){};
 };
 
 class Parser {
 public:
     static Parser& GetInstance();
-    void DumpState() const;
     bool ParseConfig(const std::string& path);
     void AddSectionParser(const std::string& name,
                           std::unique_ptr<SectionParser> parser);
diff --git a/init/service.cpp b/init/service.cpp
index c0745e3..08d0a1d 100644
--- a/init/service.cpp
+++ b/init/service.cpp
@@ -877,11 +877,6 @@
 }
 
 void ServiceManager::AddService(std::unique_ptr<Service> service) {
-    Service* old_service = FindServiceByName(service->name());
-    if (old_service) {
-        LOG(ERROR) << "ignored duplicate definition of service '" << service->name() << "'";
-        return;
-    }
     services_.emplace_back(std::move(service));
 }
 
@@ -1095,7 +1090,7 @@
     }
 }
 
-bool ServiceParser::ParseSection(const std::vector<std::string>& args, const std::string& filename,
+bool ServiceParser::ParseSection(std::vector<std::string>&& args, const std::string& filename,
                                  int line, std::string* err) {
     if (args.size() < 3) {
         *err = "services must have a name and a program";
@@ -1108,19 +1103,24 @@
         return false;
     }
 
+    Service* old_service = service_manager_->FindServiceByName(name);
+    if (old_service) {
+        *err = "ignored duplicate definition of service '" + name + "'";
+        return false;
+    }
+
     std::vector<std::string> str_args(args.begin() + 2, args.end());
     service_ = std::make_unique<Service>(name, str_args);
     return true;
 }
 
-bool ServiceParser::ParseLineSection(const std::vector<std::string>& args, int line,
-                                     std::string* err) {
-    return service_ ? service_->ParseLine(args, err) : false;
+bool ServiceParser::ParseLineSection(std::vector<std::string>&& args, int line, std::string* err) {
+    return service_ ? service_->ParseLine(std::move(args), err) : false;
 }
 
 void ServiceParser::EndSection() {
     if (service_) {
-        ServiceManager::GetInstance().AddService(std::move(service_));
+        service_manager_->AddService(std::move(service_));
     }
 }
 
diff --git a/init/service.h b/init/service.h
index 6fe0258..634fe4e 100644
--- a/init/service.h
+++ b/init/service.h
@@ -213,16 +213,17 @@
 
 class ServiceParser : public SectionParser {
   public:
-    ServiceParser() : service_(nullptr) {}
-    bool ParseSection(const std::vector<std::string>& args, const std::string& filename, int line,
+    ServiceParser(ServiceManager* service_manager)
+        : service_manager_(service_manager), service_(nullptr) {}
+    bool ParseSection(std::vector<std::string>&& args, const std::string& filename, int line,
                       std::string* err) override;
-    bool ParseLineSection(const std::vector<std::string>& args, int line, std::string* err) override;
+    bool ParseLineSection(std::vector<std::string>&& args, int line, std::string* err) override;
     void EndSection() override;
-    void EndFile(const std::string&) override {}
 
   private:
     bool IsValidName(const std::string& name) const;
 
+    ServiceManager* service_manager_;
     std::unique_ptr<Service> service_;
 };