Revert "Revert "Remove boolean setter callbacks for Cuttlefish flag library""
This reverts commit 722663e513d06d32803246babf04594279fdee8d.
Bug: 285955928
Test: stop_cvd --helpxml
Change-Id: I03c7f96ffd904cbedc39b54f99f1f6077e6eb83c
diff --git a/common/libs/utils/flag_parser.cpp b/common/libs/utils/flag_parser.cpp
index 2a62dd8..4e821d7 100644
--- a/common/libs/utils/flag_parser.cpp
+++ b/common/libs/utils/flag_parser.cpp
@@ -123,27 +123,6 @@
return *this;
}
-Flag& Flag::Setter(std::function<bool(const FlagMatch&)> fn) & {
- setter_ = [fn = std::move(fn)](const FlagMatch& match) -> Result<void> {
- if (fn(match)) {
- return {};
- } else {
- return CF_ERR("Flag setter failed");
- }
- };
- return *this;
-}
-Flag Flag::Setter(std::function<bool(const FlagMatch&)> fn) && {
- setter_ = [fn = std::move(fn)](const FlagMatch& match) -> Result<void> {
- if (fn(match)) {
- return {};
- } else {
- return CF_ERR("Flag setter failed");
- }
- };
- return *this;
-}
-
Flag& Flag::Setter(std::function<Result<void>(const FlagMatch&)> setter) & {
setter_ = std::move(setter);
return *this;
@@ -440,14 +419,14 @@
}
Flag HelpFlag(const std::vector<Flag>& flags, std::string text) {
- auto setter = [&flags, text](FlagMatch) {
+ auto setter = [&flags, text](FlagMatch) -> Result<void> {
if (text.size() > 0) {
LOG(INFO) << text;
}
for (const auto& flag : flags) {
LOG(INFO) << flag;
}
- return false;
+ return CF_ERR("user requested early exit");
};
return Flag()
.Alias({FlagAliasMode::kFlagExact, "-help"})
@@ -518,9 +497,8 @@
.Help(
"This executable only supports the flags in `-help`. Positional "
"arguments may be supported.")
- .Setter([](const FlagMatch& match) {
- LOG(ERROR) << "Unknown flag " << match.value;
- return false;
+ .Setter([](const FlagMatch& match) -> Result<void> {
+ return CF_ERRF("Unknown flag \"{}\"", match.value);
});
}
@@ -530,9 +508,8 @@
.Help(
"This executable only supports the flags in `-help`. Positional "
"arguments are not supported.")
- .Setter([](const FlagMatch& match) {
- LOG(ERROR) << "Unexpected argument \"" << match.value << "\"";
- return false;
+ .Setter([](const FlagMatch& match) -> Result<void> {
+ return CF_ERRF("Unexpected argument \"{}\"", match.value);
});
}
@@ -547,9 +524,9 @@
Flag GflagsCompatFlag(const std::string& name, std::string& value) {
return GflagsCompatFlag(name)
.Getter([&value]() { return value; })
- .Setter([&value](const FlagMatch& match) {
+ .Setter([&value](const FlagMatch& match) -> Result<void> {
value = match.value;
- return true;
+ return {};
});
}
diff --git a/common/libs/utils/flag_parser.h b/common/libs/utils/flag_parser.h
index efc8c7d..3304311 100644
--- a/common/libs/utils/flag_parser.h
+++ b/common/libs/utils/flag_parser.h
@@ -81,9 +81,8 @@
/* Set a loader that displays the current value in help text. Optional. */
Flag& Getter(std::function<std::string()>) &;
Flag Getter(std::function<std::string()>) &&;
- /* Set the callback for matches. The callback be invoked multiple times. */
- Flag& Setter(std::function<bool(const FlagMatch&)>) &;
- Flag Setter(std::function<bool(const FlagMatch&)>) &&;
+ /* Set the callback for matches. The callback may be invoked multiple times.
+ */
Flag& Setter(std::function<Result<void>(const FlagMatch&)>) &;
Flag Setter(std::function<Result<void>(const FlagMatch&)>) &&;
diff --git a/common/libs/utils/flag_parser_test.cpp b/common/libs/utils/flag_parser_test.cpp
index 74422e5..0da8471 100644
--- a/common/libs/utils/flag_parser_test.cpp
+++ b/common/libs/utils/flag_parser_test.cpp
@@ -117,9 +117,9 @@
TEST(FlagParser, RepeatedListFlag) {
std::vector<std::string> elems;
auto flag = GflagsCompatFlag("myflag");
- flag.Setter([&elems](const FlagMatch& match) {
+ flag.Setter([&elems](const FlagMatch& match) -> Result<void> {
elems.push_back(match.value);
- return true;
+ return {};
});
ASSERT_THAT(flag.Parse({"-myflag=a", "--myflag", "b"}), IsOk());
ASSERT_EQ(elems, (std::vector<std::string>{"a", "b"}));
@@ -391,9 +391,9 @@
elems_.clear();
flag_ = Flag()
.Alias({FlagAliasMode::kFlagConsumesArbitrary, "--flag"})
- .Setter([this](const FlagMatch& match) {
+ .Setter([this](const FlagMatch& match) -> Result<void> {
elems_.push_back(match.value);
- return true;
+ return {};
});
}
Flag flag_;
diff --git a/common/libs/utils/shared_fd_flag.cpp b/common/libs/utils/shared_fd_flag.cpp
index b894147..e50d6d1 100644
--- a/common/libs/utils/shared_fd_flag.cpp
+++ b/common/libs/utils/shared_fd_flag.cpp
@@ -31,18 +31,16 @@
namespace cuttlefish {
-static bool Set(const FlagMatch& match, SharedFD& out) {
+static Result<void> Set(const FlagMatch& match, SharedFD& out) {
int raw_fd;
- if (!android::base::ParseInt(match.value.c_str(), &raw_fd)) {
- LOG(ERROR) << "Failed to parse value \"" << match.value
- << "\" for fd flag \"" << match.key << "\"";
- return false;
- }
+ CF_EXPECTF(android::base::ParseInt(match.value.c_str(), &raw_fd),
+ "Failed to parse value \"{}\" for fd flag \"{}\"", match.value,
+ match.key);
out = SharedFD::Dup(raw_fd);
if (out->IsOpen()) {
close(raw_fd);
}
- return true;
+ return {};
}
Flag SharedFDFlag(SharedFD& out) {
diff --git a/host/commands/cvd/acloud/converter.cpp b/host/commands/cvd/acloud/converter.cpp
index 6c90b6e..b3638d5 100644
--- a/host/commands/cvd/acloud/converter.cpp
+++ b/host/commands/cvd/acloud/converter.cpp
@@ -182,36 +182,36 @@
Flag()
.Alias({FlagAliasMode::kFlagConsumesFollowing, "--boot-build-id"})
.Alias({FlagAliasMode::kFlagConsumesFollowing, "--boot_build_id"})
- .Setter([&boot_build_id](const FlagMatch& m) {
+ .Setter([&boot_build_id](const FlagMatch& m) -> Result<void> {
boot_build_id = m.value;
- return true;
+ return {};
}));
std::optional<std::string> boot_build_target;
flags.emplace_back(
Flag()
.Alias({FlagAliasMode::kFlagConsumesFollowing, "--boot-build-target"})
.Alias({FlagAliasMode::kFlagConsumesFollowing, "--boot_build_target"})
- .Setter([&boot_build_target](const FlagMatch& m) {
+ .Setter([&boot_build_target](const FlagMatch& m) -> Result<void> {
boot_build_target = m.value;
- return true;
+ return {};
}));
std::optional<std::string> boot_branch;
flags.emplace_back(
Flag()
.Alias({FlagAliasMode::kFlagConsumesFollowing, "--boot-branch"})
.Alias({FlagAliasMode::kFlagConsumesFollowing, "--boot_branch"})
- .Setter([&boot_branch](const FlagMatch& m) {
+ .Setter([&boot_branch](const FlagMatch& m) -> Result<void> {
boot_branch = m.value;
- return true;
+ return {};
}));
std::optional<std::string> boot_artifact;
flags.emplace_back(
Flag()
.Alias({FlagAliasMode::kFlagConsumesFollowing, "--boot-artifact"})
.Alias({FlagAliasMode::kFlagConsumesFollowing, "--boot_artifact"})
- .Setter([&boot_artifact](const FlagMatch& m) {
+ .Setter([&boot_artifact](const FlagMatch& m) -> Result<void> {
boot_artifact = m.value;
- return true;
+ return {};
}));
std::optional<std::string> ota_build_id;
@@ -219,90 +219,92 @@
Flag()
.Alias({FlagAliasMode::kFlagConsumesFollowing, "--ota-build-id"})
.Alias({FlagAliasMode::kFlagConsumesFollowing, "--ota_build_id"})
- .Setter([&ota_build_id](const FlagMatch& m) {
+ .Setter([&ota_build_id](const FlagMatch& m) -> Result<void> {
ota_build_id = m.value;
- return true;
+ return {};
}));
std::optional<std::string> ota_build_target;
flags.emplace_back(
Flag()
.Alias({FlagAliasMode::kFlagConsumesFollowing, "--ota-build-target"})
.Alias({FlagAliasMode::kFlagConsumesFollowing, "--ota_build_target"})
- .Setter([&ota_build_target](const FlagMatch& m) {
+ .Setter([&ota_build_target](const FlagMatch& m) -> Result<void> {
ota_build_target = m.value;
- return true;
+ return {};
}));
std::optional<std::string> ota_branch;
flags.emplace_back(
Flag()
.Alias({FlagAliasMode::kFlagConsumesFollowing, "--ota-branch"})
.Alias({FlagAliasMode::kFlagConsumesFollowing, "--ota_branch"})
- .Setter([&ota_branch](const FlagMatch& m) {
+ .Setter([&ota_branch](const FlagMatch& m) -> Result<void> {
ota_branch = m.value;
- return true;
+ return {};
}));
std::optional<std::string> launch_args;
flags.emplace_back(
Flag()
.Alias({FlagAliasMode::kFlagConsumesFollowing, "--launch-args"})
- .Setter([&launch_args](const FlagMatch& m) {
+ .Setter([&launch_args](const FlagMatch& m) -> Result<void> {
launch_args = m.value;
- return true;
+ return {};
}));
std::optional<std::string> system_branch;
flags.emplace_back(
Flag()
.Alias({FlagAliasMode::kFlagConsumesFollowing, "--system-branch"})
- .Setter([&system_branch](const FlagMatch& m) {
+ .Setter([&system_branch](const FlagMatch& m) -> Result<void> {
system_branch = m.value;
- return true;
+ return {};
}));
std::optional<std::string> system_build_target;
- flags.emplace_back(Flag()
- .Alias({FlagAliasMode::kFlagConsumesFollowing,
- "--system-build-target"})
- .Setter([&system_build_target](const FlagMatch& m) {
- system_build_target = m.value;
- return true;
- }));
+ flags.emplace_back(
+ Flag()
+ .Alias(
+ {FlagAliasMode::kFlagConsumesFollowing, "--system-build-target"})
+ .Setter([&system_build_target](const FlagMatch& m) -> Result<void> {
+ system_build_target = m.value;
+ return {};
+ }));
std::optional<std::string> system_build_id;
flags.emplace_back(
Flag()
.Alias({FlagAliasMode::kFlagConsumesFollowing, "--system-build-id"})
- .Setter([&system_build_id](const FlagMatch& m) {
+ .Setter([&system_build_id](const FlagMatch& m) -> Result<void> {
system_build_id = m.value;
- return true;
+ return {};
}));
std::optional<std::string> kernel_branch;
flags.emplace_back(
Flag()
.Alias({FlagAliasMode::kFlagConsumesFollowing, "--kernel-branch"})
- .Setter([&kernel_branch](const FlagMatch& m) {
+ .Setter([&kernel_branch](const FlagMatch& m) -> Result<void> {
kernel_branch = m.value;
- return true;
+ return {};
}));
std::optional<std::string> kernel_build_target;
- flags.emplace_back(Flag()
- .Alias({FlagAliasMode::kFlagConsumesFollowing,
- "--kernel-build-target"})
- .Setter([&kernel_build_target](const FlagMatch& m) {
- kernel_build_target = m.value;
- return true;
- }));
+ flags.emplace_back(
+ Flag()
+ .Alias(
+ {FlagAliasMode::kFlagConsumesFollowing, "--kernel-build-target"})
+ .Setter([&kernel_build_target](const FlagMatch& m) -> Result<void> {
+ kernel_build_target = m.value;
+ return {};
+ }));
std::optional<std::string> kernel_build_id;
flags.emplace_back(
Flag()
.Alias({FlagAliasMode::kFlagConsumesFollowing, "--kernel-build-id"})
- .Setter([&kernel_build_id](const FlagMatch& m) {
+ .Setter([&kernel_build_id](const FlagMatch& m) -> Result<void> {
kernel_build_id = m.value;
- return true;
+ return {};
}));
bool use_16k = false;
flags.emplace_back(Flag()
@@ -310,9 +312,9 @@
.Alias({FlagAliasMode::kFlagExact, "--16K"})
.Alias({FlagAliasMode::kFlagExact, "--use-16k"})
.Alias({FlagAliasMode::kFlagExact, "--use-16K"})
- .Setter([&use_16k](const FlagMatch&) {
+ .Setter([&use_16k](const FlagMatch&) -> Result<void> {
use_16k = true;
- return true;
+ return {};
}));
std::optional<std::string> pet_name;
@@ -320,9 +322,9 @@
flags.emplace_back(
GflagsCompatFlag("pet-name")
.Getter([&pet_name]() { return (pet_name ? *pet_name : ""); })
- .Setter([&pet_name](const FlagMatch& match) {
+ .Setter([&pet_name](const FlagMatch& match) -> Result<void> {
pet_name = match.value;
- return true;
+ return {};
}));
CF_EXPECT(ParseFlags(flags, arguments));
diff --git a/host/commands/cvd/acloud/converter_parser_common.h b/host/commands/cvd/acloud/converter_parser_common.h
index 8274145..f3422d5 100644
--- a/host/commands/cvd/acloud/converter_parser_common.h
+++ b/host/commands/cvd/acloud/converter_parser_common.h
@@ -42,9 +42,9 @@
for (const auto& alias_name : alias_names) {
new_flag.Alias({FlagAliasMode::kFlagConsumesFollowing, "--" + alias_name});
}
- new_flag.Setter([&opt](const FlagMatch& m) {
+ new_flag.Setter([&opt](const FlagMatch& m) -> Result<void> {
opt = m.value;
- return true;
+ return {};
});
return new_flag;
}
diff --git a/host/commands/cvd/acloud/create_converter_parser.cpp b/host/commands/cvd/acloud/create_converter_parser.cpp
index 816e58d..ba9a527 100644
--- a/host/commands/cvd/acloud/create_converter_parser.cpp
+++ b/host/commands/cvd/acloud/create_converter_parser.cpp
@@ -32,22 +32,21 @@
auto local_instance_flag = Flag();
local_instance_flag.Alias(
{FlagAliasMode::kFlagConsumesArbitrary, "--local-instance"});
- local_instance_flag.Setter(
- [&local_instance_set, &local_instance](const FlagMatch& m) {
- local_instance_set = true;
- if (m.value != "" && local_instance) {
- LOG(ERROR) << "Instance number already set, was \"" << *local_instance
- << "\", now set to \"" << m.value << "\"";
- return false;
- } else if (m.value != "" && !local_instance) {
- int value = -1;
- if (!android::base::ParseInt(m.value, &value)) {
- return false;
- }
- local_instance = value;
- }
- return true;
- });
+ local_instance_flag.Setter([&local_instance_set, &local_instance](
+ const FlagMatch& m) -> Result<void> {
+ local_instance_set = true;
+ if (m.value != "" && local_instance) {
+ return CF_ERRF(
+ "Instance number already set, was \"{}\", now set to \"{}\"",
+ *local_instance, m.value);
+ } else if (m.value != "" && !local_instance) {
+ int value = -1;
+ CF_EXPECTF(android::base::ParseInt(m.value, &value),
+ "Failed to parse \"{}\"", m.value);
+ local_instance = value;
+ }
+ return {};
+ });
return local_instance_flag;
}
@@ -56,9 +55,9 @@
.Alias({FlagAliasMode::kFlagExact, "-v"})
.Alias({FlagAliasMode::kFlagExact, "-vv"})
.Alias({FlagAliasMode::kFlagExact, "--verbose"})
- .Setter([&verbose](const FlagMatch&) {
+ .Setter([&verbose](const FlagMatch&) -> Result<void> {
verbose = true;
- return true;
+ return {};
});
return verbose_flag;
}
@@ -67,12 +66,13 @@
std::optional<std::string>& local_image_path) {
return Flag()
.Alias({FlagAliasMode::kFlagConsumesArbitrary, "--local-image"})
- .Setter([&local_image_given, &local_image_path](const FlagMatch& m) {
+ .Setter([&local_image_given,
+ &local_image_path](const FlagMatch& m) -> Result<void> {
local_image_given = true;
if (m.value != "") {
local_image_path = m.value;
}
- return true;
+ return {};
});
}
diff --git a/host/commands/cvd/handle_reset.cpp b/host/commands/cvd/handle_reset.cpp
index 2c11a81..73b653b 100644
--- a/host/commands/cvd/handle_reset.cpp
+++ b/host/commands/cvd/handle_reset.cpp
@@ -58,19 +58,20 @@
bool is_confirmed_by_flag = false;
std::string verbosity_flag_value;
- Flag y_flag = Flag()
- .Alias({FlagAliasMode::kFlagExact, "-y"})
- .Alias({FlagAliasMode::kFlagExact, "--yes"})
- .Setter([&is_confirmed_by_flag](const FlagMatch&) {
- is_confirmed_by_flag = true;
- return true;
- });
+ Flag y_flag =
+ Flag()
+ .Alias({FlagAliasMode::kFlagExact, "-y"})
+ .Alias({FlagAliasMode::kFlagExact, "--yes"})
+ .Setter([&is_confirmed_by_flag](const FlagMatch&) -> Result<void> {
+ is_confirmed_by_flag = true;
+ return {};
+ });
Flag help_flag = Flag()
.Alias({FlagAliasMode::kFlagExact, "-h"})
.Alias({FlagAliasMode::kFlagExact, "--help"})
- .Setter([&is_help](const FlagMatch&) {
+ .Setter([&is_help](const FlagMatch&) -> Result<void> {
is_help = true;
- return true;
+ return {};
});
std::vector<Flag> flags{
GflagsCompatFlag("device-by-cvd-only", device_by_cvd_only),
diff --git a/host/commands/cvd/server_command/load_configs.cpp b/host/commands/cvd/server_command/load_configs.cpp
index f905adc..5c3e7f9 100644
--- a/host/commands/cvd/server_command/load_configs.cpp
+++ b/host/commands/cvd/server_command/load_configs.cpp
@@ -347,10 +347,10 @@
flags.emplace_back(GflagsCompatFlag("help", help));
std::vector<std::string> overrides;
FlagAlias alias = {FlagAliasMode::kFlagPrefix, "--override="};
- flags.emplace_back(
- Flag().Alias(alias).Setter([&overrides](const FlagMatch& m) {
+ flags.emplace_back(Flag().Alias(alias).Setter(
+ [&overrides](const FlagMatch& m) -> Result<void> {
overrides.push_back(m.value);
- return true;
+ return {};
}));
auto args = ParseInvocation(request.Message()).arguments;
CF_EXPECT(ParseFlags(flags, args));
diff --git a/host/commands/cvd/server_command/serial_launch.cpp b/host/commands/cvd/server_command/serial_launch.cpp
index 65cd527..b78a5a7 100644
--- a/host/commands/cvd/server_command/serial_launch.cpp
+++ b/host/commands/cvd/server_command/serial_launch.cpp
@@ -69,34 +69,31 @@
/** Returns a `Flag` object that accepts comma-separated unsigned integers. */
template <typename T>
-Flag DeviceSpecificUintFlag(const std::string& name, std::vector<T>& values,
- const RequestWithStdio& request) {
+Flag DeviceSpecificUintFlag(const std::string& name, std::vector<T>& values) {
return GflagsCompatFlag(name).Setter(
- [&request, &values](const FlagMatch& match) {
+ [&values](const FlagMatch& match) -> Result<void> {
auto parsed_values = android::base::Tokenize(match.value, ", ");
for (auto& parsed_value : parsed_values) {
std::uint32_t num = 0;
- if (!android::base::ParseUint(parsed_value, &num)) {
- constexpr char kError[] = "Failed to parse integer";
- WriteAll(request.Out(), kError, sizeof(kError));
- return false;
- }
+ CF_EXPECTF(android::base::ParseUint(parsed_value, &num),
+ "Failed to parse {} as an integer", parsed_value);
values.push_back(num);
}
- return true;
+ return {};
});
}
/** Returns a `Flag` object that accepts comma-separated strings. */
Flag DeviceSpecificStringFlag(const std::string& name,
std::vector<std::string>& values) {
- return GflagsCompatFlag(name).Setter([&values](const FlagMatch& match) {
- auto parsed_values = android::base::Tokenize(match.value, ", ");
- for (auto& parsed_value : parsed_values) {
- values.push_back(parsed_value);
- }
- return true;
- });
+ return GflagsCompatFlag(name).Setter(
+ [&values](const FlagMatch& match) -> Result<void> {
+ auto parsed_values = android::base::Tokenize(match.value, ", ");
+ for (auto& parsed_value : parsed_values) {
+ values.push_back(parsed_value);
+ }
+ return {};
+ });
}
std::string ParentDir(const uid_t uid) {
@@ -167,19 +164,19 @@
flags.emplace_back(GflagsCompatFlag("verbose", verbose));
std::vector<std::uint32_t> x_res;
- flags.emplace_back(DeviceSpecificUintFlag("x_res", x_res, request));
+ flags.emplace_back(DeviceSpecificUintFlag("x_res", x_res));
std::vector<std::uint32_t> y_res;
- flags.emplace_back(DeviceSpecificUintFlag("y_res", y_res, request));
+ flags.emplace_back(DeviceSpecificUintFlag("y_res", y_res));
std::vector<std::uint32_t> dpi;
- flags.emplace_back(DeviceSpecificUintFlag("dpi", dpi, request));
+ flags.emplace_back(DeviceSpecificUintFlag("dpi", dpi));
std::vector<std::uint32_t> cpus;
- flags.emplace_back(DeviceSpecificUintFlag("cpus", cpus, request));
+ flags.emplace_back(DeviceSpecificUintFlag("cpus", cpus));
std::vector<std::uint32_t> memory_mb;
- flags.emplace_back(DeviceSpecificUintFlag("memory_mb", memory_mb, request));
+ flags.emplace_back(DeviceSpecificUintFlag("memory_mb", memory_mb));
std::vector<std::string> setupwizard_mode;
flags.emplace_back(
@@ -207,27 +204,20 @@
auto& device_flag = flags.emplace_back();
device_flag.Alias({FlagAliasMode::kFlagPrefix, "--device="});
device_flag.Alias({FlagAliasMode::kFlagConsumesFollowing, "--device"});
- device_flag.Setter(
- [this, time, client_uid, &devices, &request](const FlagMatch& mat) {
- auto lock = lock_file_manager_.TryAcquireUnusedLock();
- if (!lock.ok()) {
- WriteAll(request.Err(), lock.error().Message());
- return false;
- } else if (!lock->has_value()) {
- constexpr char kError[] = "could not acquire instance lock";
- WriteAll(request.Err(), kError, sizeof(kError));
- return false;
- }
- int num = (*lock)->Instance();
- std::string home_dir = ParentDir(client_uid) + std::to_string(time) +
- "_" + std::to_string(num) + "/";
- devices.emplace_back(Device{
- .build = mat.value,
- .home_dir = std::move(home_dir),
- .ins_lock = std::move(**lock),
- });
- return true;
- });
+ device_flag.Setter([this, time, client_uid,
+ &devices](const FlagMatch& mat) -> Result<void> {
+ auto lock = CF_EXPECT(lock_file_manager_.TryAcquireUnusedLock());
+ CF_EXPECT(lock.has_value(), "could not acquire instance lock");
+ int num = lock->Instance();
+ std::string home_dir = ParentDir(client_uid) + std::to_string(time) +
+ "_" + std::to_string(num) + "/";
+ devices.emplace_back(Device{
+ .build = mat.value,
+ .home_dir = std::move(home_dir),
+ .ins_lock = std::move(*lock),
+ });
+ return {};
+ });
auto args = ParseInvocation(request.Message()).arguments;
for (const auto& arg : args) {
diff --git a/host/commands/display/main.cpp b/host/commands/display/main.cpp
index 87f1dab..d41ba78 100644
--- a/host/commands/display/main.cpp
+++ b/host/commands/display/main.cpp
@@ -189,9 +189,9 @@
const std::vector<Flag> remove_displays_flags = {
GflagsCompatFlag(kDisplayFlag)
.Help("Display id of a display to remove.")
- .Setter([&](const FlagMatch& match) {
+ .Setter([&](const FlagMatch& match) -> Result<void> {
displays.push_back(match.value);
- return true;
+ return {};
}),
};
auto parse_res = ParseFlags(remove_displays_flags, args);
diff --git a/host/libs/config/adb/flags.cpp b/host/libs/config/adb/flags.cpp
index 6606d34..0440f61 100644
--- a/host/libs/config/adb/flags.cpp
+++ b/host/libs/config/adb/flags.cpp
@@ -36,13 +36,14 @@
}
return modes.str().substr(1); // First comma
});
- mode_flag_.Setter([this](const FlagMatch& match) {
+ mode_flag_.Setter([this](const FlagMatch& match) -> Result<void> {
// TODO(schuffelen): Error on unknown types?
std::set<AdbMode> modes;
for (auto& mode : android::base::Split(match.value, ",")) {
modes.insert(StringToAdbMode(mode));
}
- return config_.SetModes(modes);
+ CF_EXPECT(config_.SetModes(modes));
+ return {};
});
}
diff --git a/host/libs/config/config_flag.cpp b/host/libs/config/config_flag.cpp
index 457d60d..fd5bfce 100644
--- a/host/libs/config/config_flag.cpp
+++ b/host/libs/config/config_flag.cpp
@@ -123,7 +123,10 @@
"device/google/cuttlefish/shared/config/config_*.json for possible "
"values.";
auto getter = [this]() { return config_; };
- auto setter = [this](const FlagMatch& m) { return ChooseConfig(m.value); };
+ auto setter = [this](const FlagMatch& m) -> Result<void> {
+ CF_EXPECT(ChooseConfig(m.value));
+ return {};
+ };
flag_ = GflagsCompatFlag("config").Help(help).Getter(getter).Setter(setter);
}
@@ -163,15 +166,13 @@
}
private:
- bool ChooseConfig(const std::string& name) {
- if (!config_reader_.HasConfig(name)) {
- LOG(ERROR) << "Invalid --config option '" << name << "'. Valid options: "
- << android::base::Join(config_reader_.AvailableConfigs(), ",");
- return false;
- }
+ Result<void> ChooseConfig(const std::string& name) {
+ CF_EXPECTF(config_reader_.HasConfig(name),
+ "Invalid --config option '{}'. Valid options: [{}]", name,
+ fmt::join(config_reader_.AvailableConfigs(), ","));
config_ = name;
is_default_ = false;
- return true;
+ return {};
}
std::optional<std::string> FindAndroidInfoConfig() const {
auto info_path = system_image_dir_flag_.Path() + "/android-info.txt";
diff --git a/host/libs/config/custom_actions.cpp b/host/libs/config/custom_actions.cpp
index e98c46c..87aa948 100644
--- a/host/libs/config/custom_actions.cpp
+++ b/host/libs/config/custom_actions.cpp
@@ -213,19 +213,19 @@
"empty then the custom action config will be empty as well.");
custom_action_config_flag_.Getter(
[this]() { return custom_action_config_[0]; });
- custom_action_config_flag_.Setter([this](const FlagMatch& match) {
- if (!match.value.empty() &&
- (match.value == "unset" || match.value == "\"unset\"")) {
- custom_action_config_.push_back(DefaultCustomActionConfig());
- } else if (!match.value.empty() && !FileExists(match.value)) {
- LOG(ERROR) << "custom_action_config file \"" << match.value << "\" "
- << "does not exist.";
- return false;
- } else {
- custom_action_config_.push_back(match.value);
- }
- return true;
- });
+ custom_action_config_flag_.Setter(
+ [this](const FlagMatch& match) -> Result<void> {
+ if (!match.value.empty() &&
+ (match.value == "unset" || match.value == "\"unset\"")) {
+ custom_action_config_.push_back(DefaultCustomActionConfig());
+ } else if (!match.value.empty() && !FileExists(match.value)) {
+ return CF_ERRF("custom_action_config file \"{}\" does not exist.",
+ match.value);
+ } else {
+ custom_action_config_.push_back(match.value);
+ }
+ return {};
+ });
// TODO(schuffelen): Access ConfigFlag directly for these values.
custom_actions_flag_ = GflagsCompatFlag("custom_actions");
custom_actions_flag_.Help(
@@ -234,23 +234,16 @@
"preset config files; prefer --custom_action_config to specify a "
"custom config file on the command line. Actions in this flag are "
"combined with actions in --custom_action_config.");
- custom_actions_flag_.Setter([this](const FlagMatch& match) {
+ custom_actions_flag_.Setter([this](const FlagMatch& match) -> Result<void> {
// Load the custom action from the --config preset file.
if (match.value == "unset" || match.value == "\"unset\"") {
AddEmptyJsonCustomActionConfigs();
- return true;
+ return {};
}
- Json::CharReaderBuilder builder;
- std::unique_ptr<Json::CharReader> reader(builder.newCharReader());
- std::string errorMessage;
- Json::Value custom_action_array(Json::arrayValue);
- if (!reader->parse(&*match.value.begin(), &*match.value.end(),
- &custom_action_array, &errorMessage)) {
- LOG(ERROR) << "Could not read custom actions config flag: "
- << errorMessage;
- return false;
- }
- return AddJsonCustomActionConfigs(custom_action_array);
+ auto custom_action_array = CF_EXPECT(
+ ParseJson(match.value), "Could not read custom actions config flag");
+ CF_EXPECT(AddJsonCustomActionConfigs(custom_action_array));
+ return {};
});
}
diff --git a/host/libs/config/display.cpp b/host/libs/config/display.cpp
index 0e5a111..6c6540d 100644
--- a/host/libs/config/display.cpp
+++ b/host/libs/config/display.cpp
@@ -111,9 +111,9 @@
.Help(kDisplayHelp),
GflagsCompatFlag(kDisplayFlag)
.Help(kDisplayHelp)
- .Setter([&](const FlagMatch& match) {
+ .Setter([&](const FlagMatch& match) -> Result<void> {
repeated_display_flag_values.push_back(match.value);
- return true;
+ return {};
}),
};
@@ -148,4 +148,4 @@
return displays_configs;
}
-} // namespace cuttlefish
\ No newline at end of file
+} // namespace cuttlefish
