Merge changes from topic "b69050941"

* changes:
  Allow a service to override another.
  Move service name duplication lookup to EndSection
  EndSection returns Result<Success>
  Fix out of date SectionParser comment.

init/action.cpp

@@ -379,10 +379,12 @@ Result<Success> ActionParser::ParseLineSection(std::vector<std::string>&& args,
     return action_ ? action_->AddCommand(std::move(args), line) : Success();
 }
 
-void ActionParser::EndSection() {
+Result<Success> ActionParser::EndSection() {
     if (action_ && action_->NumCommands() > 0) {
         action_manager_->AddAction(std::move(action_));
     }
+
+    return Success();
 }
 
 }  // namespace init

init/action.h

@@ -130,7 +130,7 @@ class ActionParser : public SectionParser {
     Result<Success> ParseSection(std::vector<std::string>&& args, const std::string& filename,
                                  int line) override;
     Result<Success> ParseLineSection(std::vector<std::string>&& args, int line) override;
-    void EndSection() override;
+    Result<Success> EndSection() override;
 
   private:
     ActionManager* action_manager_;

init/init_test.cpp

@@ -25,6 +25,7 @@
 #include "import_parser.h"
 #include "keyword_map.h"
 #include "parser.h"
+#include "service.h"
 #include "test_function_map.h"
 #include "util.h"
 
@@ -34,12 +35,13 @@ namespace init {
 using ActionManagerCommand = std::function<void(ActionManager&)>;
 
 void TestInit(const std::string& init_script_file, const TestFunctionMap& test_function_map,
-              const std::vector<ActionManagerCommand>& commands) {
+              const std::vector<ActionManagerCommand>& commands, ServiceList* service_list) {
     ActionManager am;
 
     Action::set_function_map(&test_function_map);
 
     Parser parser;
+    parser.AddSectionParser("service", std::make_unique<ServiceParser>(service_list, nullptr));
     parser.AddSectionParser("on", std::make_unique<ActionParser>(&am, nullptr));
     parser.AddSectionParser("import", std::make_unique<ImportParser>(&parser));
 
@@ -55,11 +57,11 @@ void TestInit(const std::string& init_script_file, const TestFunctionMap& test_f
 }
 
 void TestInitText(const std::string& init_script, const TestFunctionMap& test_function_map,
-                  const std::vector<ActionManagerCommand>& commands) {
+                  const std::vector<ActionManagerCommand>& commands, ServiceList* service_list) {
     TemporaryFile tf;
     ASSERT_TRUE(tf.fd != -1);
     ASSERT_TRUE(android::base::WriteStringToFd(init_script, tf.fd));
-    TestInit(tf.path, test_function_map, commands);
+    TestInit(tf.path, test_function_map, commands, service_list);
 }
 
 TEST(init, SimpleEventTrigger) {
@@ -76,7 +78,8 @@ pass_test
     ActionManagerCommand trigger_boot = [](ActionManager& am) { am.QueueEventTrigger("boot"); };
     std::vector<ActionManagerCommand> commands{trigger_boot};
 
-    TestInitText(init_script, test_function_map, commands);
+    ServiceList service_list;
+    TestInitText(init_script, test_function_map, commands, &service_list);
 
     EXPECT_TRUE(expect_true);
 }
@@ -104,7 +107,30 @@ execute_third
     ActionManagerCommand trigger_boot = [](ActionManager& am) { am.QueueEventTrigger("boot"); };
     std::vector<ActionManagerCommand> commands{trigger_boot};
 
-    TestInitText(init_script, test_function_map, commands);
+    ServiceList service_list;
+    TestInitText(init_script, test_function_map, commands, &service_list);
+}
+
+TEST(init, OverrideService) {
+    std::string init_script = R"init(
+service A something
+    class first
+
+service A something
+    class second
+    override
+
+)init";
+
+    ServiceList service_list;
+    TestInitText(init_script, TestFunctionMap(), {}, &service_list);
+    ASSERT_EQ(1, std::distance(service_list.begin(), service_list.end()));
+
+    auto service = service_list.begin()->get();
+    ASSERT_NE(nullptr, service);
+    EXPECT_EQ(std::set<std::string>({"second"}), service->classnames());
+    EXPECT_EQ("A", service->name());
+    EXPECT_TRUE(service->is_override());
 }
 
 TEST(init, EventTriggerOrderMultipleFiles) {
@@ -162,7 +188,9 @@ TEST(init, EventTriggerOrderMultipleFiles) {
     ActionManagerCommand trigger_boot = [](ActionManager& am) { am.QueueEventTrigger("boot"); };
     std::vector<ActionManagerCommand> commands{trigger_boot};
 
-    TestInit(start.path, test_function_map, commands);
+    ServiceList service_list;
+
+    TestInit(start.path, test_function_map, commands, &service_list);
 
     EXPECT_EQ(6, num_executed);
 }

init/parser.cpp

@@ -50,12 +50,24 @@ void Parser::ParseData(const std::string& filename, const std::string& data) {
     state.nexttoken = 0;
 
     SectionParser* section_parser = nullptr;
+    int section_start_line = -1;
     std::vector<std::string> args;
 
+    auto end_section = [&] {
+        if (section_parser == nullptr) return;
+
+        if (auto result = section_parser->EndSection(); !result) {
+            LOG(ERROR) << filename << ": " << section_start_line << ": " << result.error();
+        }
+
+        section_parser = nullptr;
+        section_start_line = -1;
+    };
+
     for (;;) {
         switch (next_token(&state)) {
             case T_EOF:
-                if (section_parser) section_parser->EndSection();
+                end_section();
                 return;
             case T_NEWLINE:
                 state.line++;
@@ -65,18 +77,18 @@ void Parser::ParseData(const std::string& filename, const std::string& data) {
                 // uevent.
                 for (const auto& [prefix, callback] : line_callbacks_) {
                     if (android::base::StartsWith(args[0], prefix.c_str())) {
-                        if (section_parser) section_parser->EndSection();
+                        end_section();
 
                         if (auto result = callback(std::move(args)); !result) {
                             LOG(ERROR) << filename << ": " << state.line << ": " << result.error();
                         }
-                        section_parser = nullptr;
                         break;
                     }
                 }
                 if (section_parsers_.count(args[0])) {
-                    if (section_parser) section_parser->EndSection();
+                    end_section();
                     section_parser = section_parsers_[args[0]].get();
+                    section_start_line = state.line;
                     if (auto result =
                             section_parser->ParseSection(std::move(args), filename, state.line);
                         !result) {

init/parser.h

@@ -26,24 +26,22 @@
 
 //  SectionParser is an interface that can parse a given 'section' in init.
 //
-//  You can implement up to 4 functions below, with ParseSection() being mandatory.
+//  You can implement up to 4 functions below, with ParseSection being mandatory. The first two
-//  The first two function return bool with false indicating a failure and has a std::string* err
+//  functions return Result<Success> indicating if they have an error. It will be reported along
-//  parameter into which an error string can be written.  It will be reported along with the
+//  with the filename and line number of where the error occurred.
-//  filename and line number of where the error occurred.
 //
-//  1) bool ParseSection(std::vector<std::string>&& args, const std::string& filename,
+//  1) ParseSection
-//                       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)
+//  2) ParseLineSection
 //    This function is called on each subsequent line until the next section is encountered.
 //
-//  3) bool EndSection()
+//  3) 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()
+//  4) 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.
 
@@ -56,7 +54,7 @@ class SectionParser {
     virtual Result<Success> ParseSection(std::vector<std::string>&& args,
                                          const std::string& filename, int line) = 0;
     virtual Result<Success> ParseLineSection(std::vector<std::string>&&, int) { return Success(); };
-    virtual void EndSection(){};
+    virtual Result<Success> EndSection() { return Success(); };
     virtual void EndFile(){};
 };
 

init/service.cpp

@@ -530,6 +530,11 @@ Result<Success> Service::ParseOomScoreAdjust(const std::vector<std::string>& arg
     return Success();
 }
 
+Result<Success> Service::ParseOverride(const std::vector<std::string>& args) {
+    override_ = true;
+    return Success();
+}
+
 Result<Success> Service::ParseMemcgSwappiness(const std::vector<std::string>& args) {
     if (!ParseInt(args[1], &swappiness_, 0)) {
         return Error() << "swappiness value must be equal or greater than 0";
@@ -671,6 +676,7 @@ const Service::OptionParserMap::Map& Service::OptionParserMap::map() const {
         {"keycodes",    {1,     kMax, &Service::ParseKeycodes}},
         {"oneshot",     {0,     0,    &Service::ParseOneshot}},
         {"onrestart",   {1,     kMax, &Service::ParseOnrestart}},
+        {"override",    {0,     0,    &Service::ParseOverride}},
         {"oom_score_adjust",
                         {1,     1,    &Service::ParseOomScoreAdjust}},
         {"memcg.swappiness",
@@ -1111,11 +1117,6 @@ Result<Success> ServiceParser::ParseSection(std::vector<std::string>&& args,
         return Error() << "invalid service name '" << name << "'";
     }
 
-    Service* old_service = service_list_->FindService(name);
-    if (old_service) {
-        return Error() << "ignored duplicate definition of service '" << name << "'";
-    }
-
     Subcontext* restart_action_subcontext = nullptr;
     if (subcontexts_) {
         for (auto& subcontext : *subcontexts_) {
@@ -1135,10 +1136,23 @@ Result<Success> ServiceParser::ParseLineSection(std::vector<std::string>&& args,
     return service_ ? service_->ParseLine(std::move(args)) : Success();
 }
 
-void ServiceParser::EndSection() {
+Result<Success> ServiceParser::EndSection() {
     if (service_) {
+        Service* old_service = service_list_->FindService(service_->name());
+        if (old_service) {
+            if (!service_->is_override()) {
+                return Error() << "ignored duplicate definition of service '" << service_->name()
+                               << "'";
+            }
+
+            service_list_->RemoveService(*old_service);
+            old_service = nullptr;
+        }
+
         service_list_->AddService(std::move(service_));
     }
+
+    return Success();
 }
 
 bool ServiceParser::IsValidName(const std::string& name) const {

init/service.h

@@ -111,6 +111,7 @@ class Service {
     const std::set<std::string>& interfaces() const { return interfaces_; }
     int priority() const { return priority_; }
     int oom_score_adjust() const { return oom_score_adjust_; }
+    bool is_override() const { return override_; }
     bool process_cgroup_empty() const { return process_cgroup_empty_; }
     unsigned long start_order() const { return start_order_; }
     const std::vector<std::string>& args() const { return args_; }
@@ -139,6 +140,7 @@ class Service {
     Result<Success> ParseOneshot(const std::vector<std::string>& args);
     Result<Success> ParseOnrestart(const std::vector<std::string>& args);
     Result<Success> ParseOomScoreAdjust(const std::vector<std::string>& args);
+    Result<Success> ParseOverride(const std::vector<std::string>& args);
     Result<Success> ParseMemcgLimitInBytes(const std::vector<std::string>& args);
     Result<Success> ParseMemcgSoftLimitInBytes(const std::vector<std::string>& args);
     Result<Success> ParseMemcgSwappiness(const std::vector<std::string>& args);
@@ -201,6 +203,8 @@ class Service {
 
     bool process_cgroup_empty_ = false;
 
+    bool override_ = false;
+
     unsigned long start_order_;
 
     std::vector<std::pair<int, rlimit>> rlimits_;
@@ -248,7 +252,7 @@ class ServiceParser : public SectionParser {
     Result<Success> ParseSection(std::vector<std::string>&& args, const std::string& filename,
                                  int line) override;
     Result<Success> ParseLineSection(std::vector<std::string>&& args, int line) override;
-    void EndSection() override;
+    Result<Success> EndSection() override;
 
   private:
     bool IsValidName(const std::string& name) const;

init/ueventd_parser.cpp

@@ -132,8 +132,10 @@ Result<Success> SubsystemParser::ParseLineSection(std::vector<std::string>&& arg
     return std::invoke(*parser, this, std::move(args));
 }
 
-void SubsystemParser::EndSection() {
+Result<Success> SubsystemParser::EndSection() {
     subsystems_->emplace_back(std::move(subsystem_));
+
+    return Success();
 }
 
 }  // namespace init

init/ueventd_parser.h

@@ -32,7 +32,7 @@ class SubsystemParser : public SectionParser {
     Result<Success> ParseSection(std::vector<std::string>&& args, const std::string& filename,
                                  int line) override;
     Result<Success> ParseLineSection(std::vector<std::string>&& args, int line) override;
-    void EndSection() override;
+    Result<Success> EndSection() override;
 
   private:
     Result<Success> ParseDevName(std::vector<std::string>&& args);