init: simplify keyword_map
I've heard that keyword_map is too complex, in particular the tuple
and the pair in BuiltinFunctionMap, so this change removes a lot of
that complexity and, more importantly, better documents how all of
this works.
Test: boot, init unit tests
Change-Id: I74e5f9de7f2ec524cb6127bb9da2956b5f307f56
diff --git a/init/action.cpp b/init/action.cpp
index 8cf7645..0c476df 100644
--- a/init/action.cpp
+++ b/init/action.cpp
@@ -80,17 +80,20 @@
filename_(filename),
line_(line) {}
-const KeywordFunctionMap* Action::function_map_ = nullptr;
+const BuiltinFunctionMap* Action::function_map_ = nullptr;
Result<void> Action::AddCommand(std::vector<std::string>&& args, int line) {
if (!function_map_) {
return Error() << "no function map available";
}
- auto function = function_map_->FindFunction(args);
- if (!function) return Error() << function.error();
+ auto map_result = function_map_->Find(args);
+ if (!map_result) {
+ return Error() << map_result.error();
+ }
- commands_.emplace_back(function->second, function->first, std::move(args), line);
+ commands_.emplace_back(map_result->function, map_result->run_in_subcontext, std::move(args),
+ line);
return {};
}
diff --git a/init/action.h b/init/action.h
index 13b250a..80c1da4 100644
--- a/init/action.h
+++ b/init/action.h
@@ -75,7 +75,7 @@
bool oneshot() const { return oneshot_; }
const std::string& filename() const { return filename_; }
int line() const { return line_; }
- static void set_function_map(const KeywordFunctionMap* function_map) {
+ static void set_function_map(const BuiltinFunctionMap* function_map) {
function_map_ = function_map;
}
@@ -91,7 +91,7 @@
Subcontext* subcontext_;
std::string filename_;
int line_;
- static const KeywordFunctionMap* function_map_;
+ static const BuiltinFunctionMap* function_map_;
};
} // namespace init
diff --git a/init/builtins.cpp b/init/builtins.cpp
index ceab568..b6e26a1 100644
--- a/init/builtins.cpp
+++ b/init/builtins.cpp
@@ -1152,10 +1152,10 @@
}
// Builtin-function-map start
-const BuiltinFunctionMap::Map& BuiltinFunctionMap::map() const {
+const BuiltinFunctionMap& GetBuiltinFunctionMap() {
constexpr std::size_t kMax = std::numeric_limits<std::size_t>::max();
// clang-format off
- static const Map builtin_functions = {
+ static const BuiltinFunctionMap builtin_functions = {
{"bootchart", {1, 1, {false, do_bootchart}}},
{"chmod", {2, 2, {true, do_chmod}}},
{"chown", {2, 3, {true, do_chown}}},
diff --git a/init/builtins.h b/init/builtins.h
index 7bbf6aa..f0ff1eb 100644
--- a/init/builtins.h
+++ b/init/builtins.h
@@ -14,8 +14,7 @@
* limitations under the License.
*/
-#ifndef _INIT_BUILTINS_H
-#define _INIT_BUILTINS_H
+#pragma once
#include <functional>
#include <map>
@@ -31,18 +30,16 @@
using BuiltinFunction = std::function<Result<void>(const BuiltinArguments&)>;
-using KeywordFunctionMap = KeywordMap<std::pair<bool, BuiltinFunction>>;
-class BuiltinFunctionMap : public KeywordFunctionMap {
- public:
- BuiltinFunctionMap() {}
-
- private:
- const Map& map() const override;
+struct BuiltinFunctionMapValue {
+ bool run_in_subcontext;
+ BuiltinFunction function;
};
+using BuiltinFunctionMap = KeywordMap<BuiltinFunctionMapValue>;
+
+const BuiltinFunctionMap& GetBuiltinFunctionMap();
+
extern std::vector<std::string> late_import_paths;
} // namespace init
} // namespace android
-
-#endif
diff --git a/init/host_init_verifier.cpp b/init/host_init_verifier.cpp
index 92c2aa5..dfde51b 100644
--- a/init/host_init_verifier.cpp
+++ b/init/host_init_verifier.cpp
@@ -221,7 +221,7 @@
return EXIT_FAILURE;
}
- const BuiltinFunctionMap function_map;
+ const BuiltinFunctionMap& function_map = GetBuiltinFunctionMap();
Action::set_function_map(&function_map);
ActionManager& am = ActionManager::GetInstance();
ServiceList& sl = ServiceList::GetInstance();
diff --git a/init/init.cpp b/init/init.cpp
index 5dba54d..18fb0c3 100644
--- a/init/init.cpp
+++ b/init/init.cpp
@@ -688,7 +688,7 @@
MountHandler mount_handler(&epoll);
set_usb_controller();
- const BuiltinFunctionMap function_map;
+ const BuiltinFunctionMap& function_map = GetBuiltinFunctionMap();
Action::set_function_map(&function_map);
if (!SetupMountNamespaces()) {
diff --git a/init/init_test.cpp b/init/init_test.cpp
index a09db18..0411214 100644
--- a/init/init_test.cpp
+++ b/init/init_test.cpp
@@ -22,6 +22,7 @@
#include "action.h"
#include "action_manager.h"
#include "action_parser.h"
+#include "builtin_arguments.h"
#include "builtins.h"
#include "import_parser.h"
#include "keyword_map.h"
@@ -29,7 +30,6 @@
#include "service.h"
#include "service_list.h"
#include "service_parser.h"
-#include "test_function_map.h"
#include "util.h"
namespace android {
@@ -37,7 +37,7 @@
using ActionManagerCommand = std::function<void(ActionManager&)>;
-void TestInit(const std::string& init_script_file, const TestFunctionMap& test_function_map,
+void TestInit(const std::string& init_script_file, const BuiltinFunctionMap& test_function_map,
const std::vector<ActionManagerCommand>& commands, ServiceList* service_list) {
ActionManager am;
@@ -60,7 +60,7 @@
}
}
-void TestInitText(const std::string& init_script, const TestFunctionMap& test_function_map,
+void TestInitText(const std::string& init_script, const BuiltinFunctionMap& test_function_map,
const std::vector<ActionManagerCommand>& commands, ServiceList* service_list) {
TemporaryFile tf;
ASSERT_TRUE(tf.fd != -1);
@@ -76,8 +76,13 @@
pass_test
)init";
- TestFunctionMap test_function_map;
- test_function_map.Add("pass_test", [&expect_true]() { expect_true = true; });
+ auto do_pass_test = [&expect_true](const BuiltinArguments&) {
+ expect_true = true;
+ return Result<void>{};
+ };
+ BuiltinFunctionMap test_function_map = {
+ {"pass_test", {0, 0, {false, do_pass_test}}},
+ };
ActionManagerCommand trigger_boot = [](ActionManager& am) { am.QueueEventTrigger("boot"); };
std::vector<ActionManagerCommand> commands{trigger_boot};
@@ -103,10 +108,24 @@
)init";
int num_executed = 0;
- TestFunctionMap test_function_map;
- test_function_map.Add("execute_first", [&num_executed]() { EXPECT_EQ(0, num_executed++); });
- test_function_map.Add("execute_second", [&num_executed]() { EXPECT_EQ(1, num_executed++); });
- test_function_map.Add("execute_third", [&num_executed]() { EXPECT_EQ(2, num_executed++); });
+ auto do_execute_first = [&num_executed](const BuiltinArguments&) {
+ EXPECT_EQ(0, num_executed++);
+ return Result<void>{};
+ };
+ auto do_execute_second = [&num_executed](const BuiltinArguments&) {
+ EXPECT_EQ(1, num_executed++);
+ return Result<void>{};
+ };
+ auto do_execute_third = [&num_executed](const BuiltinArguments&) {
+ EXPECT_EQ(2, num_executed++);
+ return Result<void>{};
+ };
+
+ BuiltinFunctionMap test_function_map = {
+ {"execute_first", {0, 0, {false, do_execute_first}}},
+ {"execute_second", {0, 0, {false, do_execute_second}}},
+ {"execute_third", {0, 0, {false, do_execute_third}}},
+ };
ActionManagerCommand trigger_boot = [](ActionManager& am) { am.QueueEventTrigger("boot"); };
std::vector<ActionManagerCommand> commands{trigger_boot};
@@ -127,7 +146,7 @@
)init";
ServiceList service_list;
- TestInitText(init_script, TestFunctionMap(), {}, &service_list);
+ TestInitText(init_script, BuiltinFunctionMap(), {}, &service_list);
ASSERT_EQ(1, std::distance(service_list.begin(), service_list.end()));
auto service = service_list.begin()->get();
@@ -186,8 +205,9 @@
return Result<void>{};
};
- TestFunctionMap test_function_map;
- test_function_map.Add("execute", 1, 1, false, execute_command);
+ BuiltinFunctionMap test_function_map = {
+ {"execute", {1, 1, {false, execute_command}}},
+ };
ActionManagerCommand trigger_boot = [](ActionManager& am) { am.QueueEventTrigger("boot"); };
std::vector<ActionManagerCommand> commands{trigger_boot};
diff --git a/init/keyword_map.h b/init/keyword_map.h
index 7837bb3..d92678f 100644
--- a/init/keyword_map.h
+++ b/init/keyword_map.h
@@ -18,36 +18,49 @@
#include <map>
#include <string>
+#include <vector>
#include "result.h"
namespace android {
namespace init {
-template <typename Function>
+// Every init builtin, init service option, and ueventd option has a minimum and maximum number of
+// arguments. These must be checked both at run time for safety and also at build time for
+// correctness in host_init_verifier. Instead of copying and pasting the boiler plate code that
+// does this check into each function, it is abstracted in KeywordMap<>. This class maps keywords
+// to functions and checks that the number of arguments provided falls in the correct range or
+// returns an error otherwise.
+
+// Value is the return value of Find(), which is typically either a single function or a struct with
+// additional information.
+template <typename Value>
class KeywordMap {
public:
- using FunctionInfo = std::tuple<std::size_t, std::size_t, Function>;
- using Map = std::map<std::string, FunctionInfo>;
+ struct MapValue {
+ size_t min_args;
+ size_t max_args;
+ Value value;
+ };
- virtual ~KeywordMap() {
- }
+ KeywordMap() {}
+ KeywordMap(std::initializer_list<std::pair<const std::string, MapValue>> init) : map_(init) {}
- const Result<Function> FindFunction(const std::vector<std::string>& args) const {
+ Result<Value> Find(const std::vector<std::string>& args) const {
if (args.empty()) return Error() << "Keyword needed, but not provided";
auto& keyword = args[0];
auto num_args = args.size() - 1;
- auto function_info_it = map().find(keyword);
- if (function_info_it == map().end()) {
+ auto result_it = map_.find(keyword);
+ if (result_it == map_.end()) {
return Errorf("Invalid keyword '{}'", keyword);
}
- auto function_info = function_info_it->second;
+ auto result = result_it->second;
- auto min_args = std::get<0>(function_info);
- auto max_args = std::get<1>(function_info);
+ auto min_args = result.min_args;
+ auto max_args = result.max_args;
if (min_args == max_args && num_args != min_args) {
return Errorf("{} requires {} argument{}", keyword, min_args,
(min_args > 1 || min_args == 0) ? "s" : "");
@@ -63,13 +76,11 @@
}
}
- return std::get<Function>(function_info);
+ return result.value;
}
private:
- // Map of keyword ->
- // (minimum number of arguments, maximum number of arguments, function pointer)
- virtual const Map& map() const = 0;
+ std::map<std::string, MapValue> map_;
};
} // namespace init
diff --git a/init/main.cpp b/init/main.cpp
index 2ce46ef..38bc74b 100644
--- a/init/main.cpp
+++ b/init/main.cpp
@@ -60,7 +60,7 @@
if (argc > 1) {
if (!strcmp(argv[1], "subcontext")) {
android::base::InitLogging(argv, &android::base::KernelLogger);
- const BuiltinFunctionMap function_map;
+ const BuiltinFunctionMap& function_map = GetBuiltinFunctionMap();
return SubcontextMain(argc, argv, &function_map);
}
diff --git a/init/service_parser.cpp b/init/service_parser.cpp
index 0fbbeb8..65d96c6 100644
--- a/init/service_parser.cpp
+++ b/init/service_parser.cpp
@@ -461,18 +461,10 @@
return {};
}
-class ServiceParser::OptionParserMap : public KeywordMap<OptionParser> {
- public:
- OptionParserMap() {}
-
- private:
- const Map& map() const override;
-};
-
-const ServiceParser::OptionParserMap::Map& ServiceParser::OptionParserMap::map() const {
+const KeywordMap<ServiceParser::OptionParser>& ServiceParser::GetParserMap() const {
constexpr std::size_t kMax = std::numeric_limits<std::size_t>::max();
// clang-format off
- static const Map option_parsers = {
+ static const KeywordMap<ServiceParser::OptionParser> parser_map = {
{"capabilities",
{0, kMax, &ServiceParser::ParseCapabilities}},
{"class", {1, kMax, &ServiceParser::ParseClass}},
@@ -518,7 +510,7 @@
{"writepid", {1, kMax, &ServiceParser::ParseWritepid}},
};
// clang-format on
- return option_parsers;
+ return parser_map;
}
Result<void> ServiceParser::ParseSection(std::vector<std::string>&& args,
@@ -561,8 +553,7 @@
return {};
}
- static const OptionParserMap parser_map;
- auto parser = parser_map.FindFunction(args);
+ auto parser = GetParserMap().Find(args);
if (!parser) return parser.error();
diff --git a/init/service_parser.h b/init/service_parser.h
index bca0739..98ab15a 100644
--- a/init/service_parser.h
+++ b/init/service_parser.h
@@ -45,7 +45,7 @@
private:
using OptionParser = Result<void> (ServiceParser::*)(std::vector<std::string>&& args);
- class OptionParserMap;
+ const KeywordMap<ServiceParser::OptionParser>& GetParserMap() const;
Result<void> ParseCapabilities(std::vector<std::string>&& args);
Result<void> ParseClass(std::vector<std::string>&& args);
diff --git a/init/subcontext.cpp b/init/subcontext.cpp
index 2f9541b..a13f0c7 100644
--- a/init/subcontext.cpp
+++ b/init/subcontext.cpp
@@ -27,6 +27,7 @@
#include <selinux/android.h>
#include "action.h"
+#include "builtins.h"
#include "util.h"
#if defined(__ANDROID__)
@@ -99,7 +100,7 @@
class SubcontextProcess {
public:
- SubcontextProcess(const KeywordFunctionMap* function_map, std::string context, int init_fd)
+ SubcontextProcess(const BuiltinFunctionMap* function_map, std::string context, int init_fd)
: function_map_(function_map), context_(std::move(context)), init_fd_(init_fd){};
void MainLoop();
@@ -109,7 +110,7 @@
void ExpandArgs(const SubcontextCommand::ExpandArgsCommand& expand_args_command,
SubcontextReply* reply) const;
- const KeywordFunctionMap* function_map_;
+ const BuiltinFunctionMap* function_map_;
const std::string context_;
const int init_fd_;
};
@@ -122,12 +123,12 @@
args.emplace_back(string);
}
- auto map_result = function_map_->FindFunction(args);
+ auto map_result = function_map_->Find(args);
Result<void> result;
if (!map_result) {
result = Error() << "Cannot find command: " << map_result.error();
} else {
- result = RunBuiltinFunction(map_result->second, args, context_);
+ result = RunBuiltinFunction(map_result->function, args, context_);
}
for (const auto& [name, value] : properties_to_set) {
@@ -215,7 +216,7 @@
} // namespace
-int SubcontextMain(int argc, char** argv, const KeywordFunctionMap* function_map) {
+int SubcontextMain(int argc, char** argv, const BuiltinFunctionMap* function_map) {
if (argc < 4) LOG(FATAL) << "Fewer than 4 args specified to subcontext (" << argc << ")";
auto context = std::string(argv[2]);
diff --git a/init/subcontext.h b/init/subcontext.h
index 16bd870..591521f 100644
--- a/init/subcontext.h
+++ b/init/subcontext.h
@@ -60,7 +60,7 @@
android::base::unique_fd socket_;
};
-int SubcontextMain(int argc, char** argv, const KeywordFunctionMap* function_map);
+int SubcontextMain(int argc, char** argv, const BuiltinFunctionMap* function_map);
std::vector<Subcontext>* InitializeSubcontexts();
bool SubcontextChildReap(pid_t pid);
void SubcontextTerminate();
diff --git a/init/subcontext_benchmark.cpp b/init/subcontext_benchmark.cpp
index fdbbc41..f6fee8a 100644
--- a/init/subcontext_benchmark.cpp
+++ b/init/subcontext_benchmark.cpp
@@ -19,8 +19,6 @@
#include <benchmark/benchmark.h>
#include <selinux/selinux.h>
-#include "test_function_map.h"
-
namespace android {
namespace init {
@@ -50,11 +48,11 @@
BENCHMARK(BenchmarkSuccess);
-TestFunctionMap BuildTestFunctionMap() {
- TestFunctionMap test_function_map;
- test_function_map.Add("return_success", 0, 0, true,
- [](const BuiltinArguments& args) { return Result<void>{}; });
-
+BuiltinFunctionMap BuildTestFunctionMap() {
+ auto function = [](const BuiltinArguments& args) { return Result<void>{}; };
+ BuiltinFunctionMap test_function_map = {
+ {"return_success", {0, 0, {true, function}}},
+ };
return test_function_map;
}
diff --git a/init/subcontext_test.cpp b/init/subcontext_test.cpp
index 55912d6..e120a62 100644
--- a/init/subcontext_test.cpp
+++ b/init/subcontext_test.cpp
@@ -26,7 +26,6 @@
#include <selinux/selinux.h>
#include "builtin_arguments.h"
-#include "test_function_map.h"
using namespace std::literals;
@@ -171,46 +170,53 @@
});
}
-TestFunctionMap BuildTestFunctionMap() {
- TestFunctionMap test_function_map;
+BuiltinFunctionMap BuildTestFunctionMap() {
// For CheckDifferentPid
- test_function_map.Add("return_pids_as_error", 0, 0, true,
- [](const BuiltinArguments& args) -> Result<void> {
- return Error() << getpid() << " " << getppid();
- });
+ auto do_return_pids_as_error = [](const BuiltinArguments& args) -> Result<void> {
+ return Error() << getpid() << " " << getppid();
+ };
// For SetProp
- test_function_map.Add("setprop", 2, 2, true, [](const BuiltinArguments& args) {
+ auto do_setprop = [](const BuiltinArguments& args) {
android::base::SetProperty(args[1], args[2]);
return Result<void>{};
- });
+ };
// For MultipleCommands
// Using a shared_ptr to extend lifetime of words to both lambdas
auto words = std::make_shared<std::vector<std::string>>();
- test_function_map.Add("add_word", 1, 1, true, [words](const BuiltinArguments& args) {
+ auto do_add_word = [words](const BuiltinArguments& args) {
words->emplace_back(args[1]);
return Result<void>{};
- });
- test_function_map.Add("return_words_as_error", 0, 0, true,
- [words](const BuiltinArguments& args) -> Result<void> {
- return Error() << Join(*words, " ");
- });
+ };
+ auto do_return_words_as_error = [words](const BuiltinArguments& args) -> Result<void> {
+ return Error() << Join(*words, " ");
+ };
// For RecoverAfterAbort
- test_function_map.Add("cause_log_fatal", 0, 0, true,
- [](const BuiltinArguments& args) -> Result<void> {
- return Error() << std::string(4097, 'f');
- });
- test_function_map.Add(
- "generate_sane_error", 0, 0, true,
- [](const BuiltinArguments& args) -> Result<void> { return Error() << "Sane error!"; });
+ auto do_cause_log_fatal = [](const BuiltinArguments& args) -> Result<void> {
+ return Error() << std::string(4097, 'f');
+ };
+ auto do_generate_sane_error = [](const BuiltinArguments& args) -> Result<void> {
+ return Error() << "Sane error!";
+ };
// For ContextString
- test_function_map.Add(
- "return_context_as_error", 0, 0, true,
- [](const BuiltinArguments& args) -> Result<void> { return Error() << args.context; });
+ auto do_return_context_as_error = [](const BuiltinArguments& args) -> Result<void> {
+ return Error() << args.context;
+ };
+ // clang-format off
+ BuiltinFunctionMap test_function_map = {
+ {"return_pids_as_error", {0, 0, {true, do_return_pids_as_error}}},
+ {"setprop", {2, 2, {true, do_setprop}}},
+ {"add_word", {1, 1, {true, do_add_word}}},
+ {"return_words_as_error", {0, 0, {true, do_return_words_as_error}}},
+ {"cause_log_fatal", {0, 0, {true, do_cause_log_fatal}}},
+ {"generate_sane_error", {0, 0, {true, do_generate_sane_error}}},
+ {"return_context_as_error", {0, 0, {true, do_return_context_as_error}}},
+ };
+ // clang-format on
return test_function_map;
}
diff --git a/init/test_function_map.h b/init/test_function_map.h
deleted file mode 100644
index 466836c..0000000
--- a/init/test_function_map.h
+++ /dev/null
@@ -1,53 +0,0 @@
-/*
- * Copyright (C) 2017 The Android Open Source Project
- *
- * Licensed under the Apache License, Version 2.0 (the "License");
- * you may not use this file except in compliance with the License.
- * You may obtain a copy of the License at
- *
- * http://www.apache.org/licenses/LICENSE-2.0
- *
- * Unless required by applicable law or agreed to in writing, software
- * distributed under the License is distributed on an "AS IS" BASIS,
- * WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied.
- * See the License for the specific language governing permissions and
- * limitations under the License.
- */
-
-#pragma once
-
-#include <string>
-#include <vector>
-
-#include "builtin_arguments.h"
-#include "builtins.h"
-#include "keyword_map.h"
-
-namespace android {
-namespace init {
-
-class TestFunctionMap : public KeywordFunctionMap {
- public:
- // Helper for argument-less functions
- using BuiltinFunctionNoArgs = std::function<void(void)>;
- void Add(const std::string& name, BuiltinFunctionNoArgs function) {
- Add(name, 0, 0, false, [f = std::move(function)](const BuiltinArguments&) {
- f();
- return Result<void>{};
- });
- }
-
- void Add(const std::string& name, std::size_t min_parameters, std::size_t max_parameters,
- bool run_in_subcontext, BuiltinFunction function) {
- builtin_functions_[name] = make_tuple(min_parameters, max_parameters,
- make_pair(run_in_subcontext, std::move(function)));
- }
-
- private:
- Map builtin_functions_ = {};
-
- const Map& map() const override { return builtin_functions_; }
-};
-
-} // namespace init
-} // namespace android
diff --git a/init/ueventd_parser.cpp b/init/ueventd_parser.cpp
index 25bab93..8ee0cce 100644
--- a/init/ueventd_parser.cpp
+++ b/init/ueventd_parser.cpp
@@ -176,21 +176,14 @@
Result<void> SubsystemParser::ParseLineSection(std::vector<std::string>&& args, int line) {
using OptionParser = Result<void> (SubsystemParser::*)(std::vector<std::string> && args);
+ // clang-format off
+ static const KeywordMap<OptionParser> parser_map = {
+ {"devname", {1, 1, &SubsystemParser::ParseDevName}},
+ {"dirname", {1, 1, &SubsystemParser::ParseDirName}},
+ };
+ // clang-format on
- static class OptionParserMap : public KeywordMap<OptionParser> {
- private:
- const Map& map() const override {
- // clang-format off
- static const Map option_parsers = {
- {"devname", {1, 1, &SubsystemParser::ParseDevName}},
- {"dirname", {1, 1, &SubsystemParser::ParseDirName}},
- };
- // clang-format on
- return option_parsers;
- }
- } parser_map;
-
- auto parser = parser_map.FindFunction(args);
+ auto parser = parser_map.Find(args);
if (!parser) return Error() << parser.error();