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
This commit is contained in:
Tom Cherry 2019-07-22 16:05:36 -07:00
parent b3fc1b7441
commit d52a5b3c10
17 changed files with 135 additions and 168 deletions

View file

@ -80,17 +80,20 @@ Action::Action(bool oneshot, Subcontext* subcontext, const std::string& filename
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 {};
}

View file

@ -75,7 +75,7 @@ class Action {
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 @@ class Action {
Subcontext* subcontext_;
std::string filename_;
int line_;
static const KeywordFunctionMap* function_map_;
static const BuiltinFunctionMap* function_map_;
};
} // namespace init

View file

@ -1152,10 +1152,10 @@ static Result<void> do_enter_default_mount_ns(const BuiltinArguments& args) {
}
// 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}}},

View file

@ -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 @@ namespace init {
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

View file

@ -221,7 +221,7 @@ int main(int argc, char** argv) {
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();

View file

@ -688,7 +688,7 @@ int SecondStageMain(int argc, char** argv) {
MountHandler mount_handler(&epoll);
set_usb_controller();
const BuiltinFunctionMap function_map;
const BuiltinFunctionMap& function_map = GetBuiltinFunctionMap();
Action::set_function_map(&function_map);
if (!SetupMountNamespaces()) {

View file

@ -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 @@ namespace init {
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 TestInit(const std::string& init_script_file, const TestFunctionMap& test_f
}
}
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 @@ on boot
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 @@ execute_third
)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 @@ service A something
)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 @@ TEST(init, EventTriggerOrderMultipleFiles) {
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};

View file

@ -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 @@ class KeywordMap {
}
}
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

View file

@ -60,7 +60,7 @@ int main(int argc, char** argv) {
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);
}

View file

@ -461,18 +461,10 @@ Result<void> ServiceParser::ParseUpdatable(std::vector<std::string>&& args) {
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 @@ const ServiceParser::OptionParserMap::Map& ServiceParser::OptionParserMap::map()
{"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 @@ Result<void> ServiceParser::ParseLineSection(std::vector<std::string>&& args, in
return {};
}
static const OptionParserMap parser_map;
auto parser = parser_map.FindFunction(args);
auto parser = GetParserMap().Find(args);
if (!parser) return parser.error();

View file

@ -45,7 +45,7 @@ class ServiceParser : public SectionParser {
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);

View file

@ -27,6 +27,7 @@
#include <selinux/android.h>
#include "action.h"
#include "builtins.h"
#include "util.h"
#if defined(__ANDROID__)
@ -99,7 +100,7 @@ uint32_t SubcontextPropertySet(const std::string& name, const std::string& value
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 @@ class SubcontextProcess {
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 @@ void SubcontextProcess::RunCommand(const SubcontextCommand::ExecuteCommand& exec
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 @@ void SubcontextProcess::MainLoop() {
} // 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]);

View file

@ -60,7 +60,7 @@ class Subcontext {
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();

View file

@ -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 @@ static void BenchmarkSuccess(benchmark::State& state) {
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;
}

View file

@ -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 @@ TEST(subcontext, ExpandArgsFailure) {
});
}
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;
}

View file

@ -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

View file

@ -176,21 +176,14 @@ Result<void> SubsystemParser::ParseDirName(std::vector<std::string>&& args) {
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();