Skip to content

Commit 57a27ae

Browse files
committed
Replace common cases of RunWithManagedStdio with a wrapper call
Bug: b/434043969
1 parent af332eb commit 57a27ae

File tree

20 files changed

+108
-198
lines changed

20 files changed

+108
-198
lines changed

base/cvd/cuttlefish/common/libs/utils/archive.cpp

Lines changed: 17 additions & 20 deletions
Original file line numberDiff line numberDiff line change
@@ -69,14 +69,9 @@ Result<std::vector<std::string>> ExtractFiles(
6969
for (const auto& extract : to_extract) {
7070
bsdtar_cmd.AddParameter(extract);
7171
}
72-
bsdtar_cmd.RedirectStdIO(Subprocess::StdIOChannel::kStdOut,
73-
Subprocess::StdIOChannel::kStdErr);
74-
std::string bsdtar_output;
75-
int bsdtar_ret = RunWithManagedStdio(std::move(bsdtar_cmd), nullptr, nullptr,
76-
&bsdtar_output);
72+
std::string bsdtar_output =
73+
CF_EXPECT(RunAndCaptureStdout(std::move(bsdtar_cmd)));
7774
LOG(DEBUG) << bsdtar_output;
78-
CF_EXPECTF(bsdtar_ret == 0, "bsdtar extraction failed on '{}', '''{}'''",
79-
archive, bsdtar_output);
8075

8176
std::vector<std::string> outputs = android::base::Split(bsdtar_output, "\n");
8277
for (std::string& output : outputs) {
@@ -135,29 +130,31 @@ std::string ExtractArchiveToMemory(const std::string& archive_filepath,
135130
bsdtar_cmd.AddParameter(archive_filepath);
136131
bsdtar_cmd.AddParameter("-O");
137132
bsdtar_cmd.AddParameter(archive_member);
138-
std::string stdout_str;
139-
auto ret =
140-
RunWithManagedStdio(std::move(bsdtar_cmd), nullptr, &stdout_str, nullptr);
141-
if (ret != 0) {
133+
Result<std::string> stdout_str = RunAndCaptureStdout(std::move(bsdtar_cmd));
134+
135+
if (!stdout_str.ok()) {
142136
LOG(ERROR) << "Could not extract \"" << archive_member << "\" from \""
143-
<< archive_filepath << "\" to memory.";
137+
<< archive_filepath
138+
<< "\" to memory: " << stdout_str.error().FormatForEnv();
144139
return "";
145140
}
146-
return stdout_str;
141+
return *stdout_str;
147142
}
148143

149144
std::vector<std::string> ArchiveContents(const std::string& archive) {
150145
Command bsdtar_cmd("/usr/bin/bsdtar");
151146
bsdtar_cmd.AddParameter("-tf");
152147
bsdtar_cmd.AddParameter(archive);
153-
std::string bsdtar_input, bsdtar_output;
154-
auto bsdtar_ret = RunWithManagedStdio(std::move(bsdtar_cmd), &bsdtar_input,
155-
&bsdtar_output, nullptr);
156-
if (bsdtar_ret != 0) {
157-
LOG(ERROR) << "`bsdtar -tf \"" << archive << "\"` returned " << bsdtar_ret;
148+
149+
Result<std::string> bsdtar_output =
150+
RunAndCaptureStdout(std::move(bsdtar_cmd));
151+
if (bsdtar_output.ok()) {
152+
return android::base::Split(*bsdtar_output, "\n");
153+
} else {
154+
LOG(ERROR) << "`bsdtar -tf '" << archive
155+
<< "'`failed: " << bsdtar_output.error().FormatForEnv();
156+
return {};
158157
}
159-
return bsdtar_ret == 0 ? android::base::Split(bsdtar_output, "\n")
160-
: std::vector<std::string>();
161158
}
162159

163160
} // namespace cuttlefish

base/cvd/cuttlefish/common/libs/utils/disk_usage.cpp

Lines changed: 1 addition & 5 deletions
Original file line numberDiff line numberDiff line change
@@ -41,11 +41,7 @@ Result<std::size_t> GetDiskUsage(const std::string& path,
4141
du_cmd.AddParameter("--block-size=" + size_arg);
4242
du_cmd.AddParameter(path);
4343

44-
std::string out;
45-
std::string err;
46-
int return_code = RunWithManagedStdio(std::move(du_cmd), nullptr, &out, &err);
47-
CF_EXPECTF(return_code == 0, "Failed to run `du` command. stderr: {}", err);
48-
CF_EXPECTF(!out.empty(), "No output read from `du` command. stderr: {}", err);
44+
std::string out = CF_EXPECT(RunAndCaptureStdout(std::move(du_cmd)));
4945
std::vector<std::string> split_out =
5046
android::base::Tokenize(out, kWhitespaceCharacters);
5147
CF_EXPECTF(!split_out.empty(),

base/cvd/cuttlefish/common/libs/utils/network.cpp

Lines changed: 1 addition & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -128,8 +128,7 @@ std::set<std::string> TapInterfacesInUse() {
128128
cmd->AddParameter(fdinfo);
129129
}
130130

131-
std::string stdout_str, stderr_str;
132-
RunWithManagedStdio(std::move(*cmd), nullptr, &stdout_str, &stderr_str);
131+
std::string stdout_str = RunAndCaptureStdout(std::move(*cmd)).value_or("");
133132

134133
auto lines = android::base::Split(stdout_str, "\n");
135134
std::set<std::string> tap_interfaces;

base/cvd/cuttlefish/common/libs/utils/wait_for_unix_socket.cpp

Lines changed: 1 addition & 8 deletions
Original file line numberDiff line numberDiff line change
@@ -80,16 +80,9 @@ Result<void> WaitForUnixSocketListeningWithoutConnect(const std::string& path,
8080
Command lsof("/usr/bin/lsof");
8181
lsof.AddParameter(/*"format"*/ "-F", /*"connection state"*/ "TST");
8282
lsof.AddParameter(path);
83-
std::string lsof_out;
84-
std::string lsof_err;
85-
int rval =
86-
RunWithManagedStdio(std::move(lsof), nullptr, &lsof_out, &lsof_err);
87-
if (rval != 0) {
88-
return CF_ERR("Failed to run `lsof`, stderr: " << lsof_err);
89-
}
83+
std::string lsof_out = CF_EXPECT(RunAndCaptureStdout(std::move(lsof)));
9084

9185
LOG(DEBUG) << "lsof stdout:|" << lsof_out << "|";
92-
LOG(DEBUG) << "lsof stderr:|" << lsof_err << "|";
9386

9487
std::smatch socket_state_match;
9588
if (std::regex_search(lsof_out, socket_state_match, socket_state_regex)) {

base/cvd/cuttlefish/host/commands/assemble_cvd/clean.cc

Lines changed: 10 additions & 11 deletions
Original file line numberDiff line numberDiff line change
@@ -104,25 +104,24 @@ Result<void> CleanPriorFiles(const std::vector<std::string>& paths,
104104
if (!InSandbox() && (!prior_dirs.empty() || !prior_files.empty())) {
105105
Command lsof("lsof");
106106
lsof.AddParameter("-t");
107+
lsof.AddParameter("-Q"); // ignore failed search terms
107108
for (const auto& prior_dir : prior_dirs) {
108109
lsof.AddParameter("+D").AddParameter(prior_dir);
109110
}
110111
for (const auto& prior_file : prior_files) {
111112
lsof.AddParameter(prior_file);
112113
}
113114

114-
std::string lsof_out;
115-
std::string lsof_err;
116-
int rval =
117-
RunWithManagedStdio(std::move(lsof), nullptr, &lsof_out, &lsof_err);
118-
if (rval != 0 && !lsof_err.empty()) {
119-
LOG(ERROR) << "Failed to run `lsof`, received message: " << lsof_err;
115+
Result<std::string> lsof_out = RunAndCaptureStdout(std::move(lsof));
116+
if (lsof_out.ok()) {
117+
std::vector<std::string> pids = android::base::Split(*lsof_out, "\n");
118+
CF_EXPECTF(
119+
lsof_out->empty(),
120+
"Instance directory files in use. Try `cvd reset`? Observed PIDs: {}",
121+
fmt::join(pids, ", "));
122+
} else {
123+
LOG(ERROR) << "Failed to run `lsof`: " << lsof_out.error().FormatForEnv();
120124
}
121-
auto pids = android::base::Split(lsof_out, "\n");
122-
CF_EXPECTF(
123-
lsof_out.empty(),
124-
"Instance directory files in use. Try `cvd reset`? Observed PIDs: {}",
125-
fmt::join(pids, ", "));
126125
}
127126

128127
for (const auto& path : paths) {

base/cvd/cuttlefish/host/commands/assemble_cvd/graphics_flags.cc

Lines changed: 8 additions & 8 deletions
Original file line numberDiff line numberDiff line change
@@ -539,7 +539,7 @@ GetGraphicsAvailabilityWithSubprocessCheck() {
539539
auto graphics_detector_binary_result = GraphicsDetectorBinaryPath();
540540
if (!graphics_detector_binary_result.ok()) {
541541
LOG(ERROR) << "Failed to run graphics detector, graphics detector path "
542-
<< " not available: "
542+
<< " not available: \n"
543543
<< graphics_detector_binary_result.error().FormatForEnv()
544544
<< ". Assuming no availability.";
545545
return {};
@@ -550,15 +550,15 @@ GetGraphicsAvailabilityWithSubprocessCheck() {
550550
Command graphics_detector_cmd(graphics_detector_binary_result.value());
551551
graphics_detector_cmd.AddParameter(graphics_availability_file.path);
552552

553-
std::string graphics_detector_stdout;
554-
auto ret = RunWithManagedStdio(std::move(graphics_detector_cmd), nullptr,
555-
&graphics_detector_stdout, nullptr);
556-
if (ret != 0) {
557-
LOG(ERROR) << "Failed to run graphics detector, bad return value: " << ret
558-
<< ". Assuming no availability.";
553+
Result<std::string> graphics_detector_stdout =
554+
RunAndCaptureStdout(std::move(graphics_detector_cmd));
555+
if (!graphics_detector_stdout.ok()) {
556+
LOG(ERROR)
557+
<< "Failed to run graphics detector, assuming no availability: \n"
558+
<< graphics_detector_stdout.error().FormatForEnv();
559559
return {};
560560
}
561-
LOG(DEBUG) << graphics_detector_stdout;
561+
LOG(DEBUG) << *graphics_detector_stdout;
562562

563563
auto graphics_availability_content_result =
564564
ReadFileContents(graphics_availability_file.path);

base/cvd/cuttlefish/host/commands/cvd/acloud/converter.cpp

Lines changed: 3 additions & 12 deletions
Original file line numberDiff line numberDiff line change
@@ -81,21 +81,15 @@ static Result<BranchBuildTargetInfo> GetDefaultBranchBuildTarget(
8181
repo_cmd.SetWorkingDirectory(fd_top);
8282
}
8383

84-
std::string repo_stdout;
85-
CF_EXPECT_EQ(
86-
RunWithManagedStdio(std::move(repo_cmd), nullptr, &repo_stdout, nullptr),
87-
0);
84+
std::string repo_stdout = CF_EXPECT(RunAndCaptureStdout(std::move(repo_cmd)));
8885

8986
Command git_cmd("git");
9087
git_cmd.AddParameter("remote");
9188
if (fd_top->IsOpen()) {
9289
git_cmd.SetWorkingDirectory(fd_top);
9390
}
9491

95-
std::string git_stdout;
96-
CF_EXPECT_EQ(
97-
RunWithManagedStdio(std::move(git_cmd), nullptr, &git_stdout, nullptr),
98-
0);
92+
std::string git_stdout = CF_EXPECT(RunAndCaptureStdout(std::move(git_cmd)));
9993

10094
git_stdout.erase(std::remove(git_stdout.begin(), git_stdout.end(), '\n'),
10195
git_stdout.cend());
@@ -133,10 +127,7 @@ Result<std::vector<std::string>> BashTokenize(const std::string& str) {
133127
command.AddParameter("-c");
134128
command.AddParameter("printf '%s\n' ", str);
135129

136-
std::string bash_stdout;
137-
CF_EXPECT(
138-
RunWithManagedStdio(std::move(command), nullptr, &bash_stdout, nullptr),
139-
0);
130+
std::string bash_stdout = CF_EXPECT(RunAndCaptureStdout(std::move(command)));
140131

141132
return android::base::Split(bash_stdout, "\n");
142133
}

base/cvd/cuttlefish/host/commands/cvd/cli/commands/BUILD.bazel

Lines changed: 0 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -454,7 +454,6 @@ cf_cc_library(
454454
"//cuttlefish/common/libs/fs",
455455
"//cuttlefish/common/libs/utils:result",
456456
"//cuttlefish/common/libs/utils:subprocess",
457-
"//cuttlefish/common/libs/utils:subprocess_managed_stdio",
458457
"//cuttlefish/host/commands/cvd/acloud",
459458
"//cuttlefish/host/commands/cvd/cli:command_request",
460459
"//cuttlefish/host/commands/cvd/cli:types",

base/cvd/cuttlefish/host/commands/cvd/cli/commands/try_acloud.cpp

Lines changed: 1 addition & 7 deletions
Original file line numberDiff line numberDiff line change
@@ -32,7 +32,6 @@
3232
#include "cuttlefish/common/libs/fs/shared_fd.h"
3333
#include "cuttlefish/common/libs/utils/result.h"
3434
#include "cuttlefish/common/libs/utils/subprocess.h"
35-
#include "cuttlefish/common/libs/utils/subprocess_managed_stdio.h"
3635
#include "cuttlefish/host/commands/cvd/acloud/config.h"
3736
#include "cuttlefish/host/commands/cvd/acloud/converter.h"
3837
#include "cuttlefish/host/commands/cvd/acloud/create_converter_parser.h"
@@ -61,12 +60,7 @@ constexpr char kDetailedHelpText[] =
6160
6261
- Or `cvdr` for remote instance management.)";
6362

64-
bool CheckIfCvdrExist() {
65-
auto cmd = Command("which").AddParameter(kCvdrBinName);
66-
int ret = RunWithManagedStdio(std::move(cmd), nullptr, nullptr, nullptr,
67-
SubprocessOptions());
68-
return ret == 0;
69-
}
63+
bool CheckIfCvdrExist() { return Execute({"which", kCvdrBinName}) == 0; }
7064

7165
} // namespace
7266

base/cvd/cuttlefish/host/commands/cvd/instances/instance_record.cpp

Lines changed: 5 additions & 12 deletions
Original file line numberDiff line numberDiff line change
@@ -104,8 +104,7 @@ Result<Json::Value> LocalInstance::FetchStatus(std::chrono::seconds timeout) {
104104
}
105105

106106
Result<void> LocalInstance::PressPowerBtn() {
107-
auto bin_check =
108-
HostToolTarget(host_artifacts_path()).GetPowerBtnBinPath();
107+
auto bin_check = HostToolTarget(host_artifacts_path()).GetPowerBtnBinPath();
109108
if (bin_check.ok()) {
110109
return PressPowerBtnLegacy();
111110
}
@@ -115,18 +114,12 @@ Result<void> LocalInstance::PressPowerBtn() {
115114
"powerbtn not supported in vm manager " << config->vm_manager());
116115
auto instance = config->ForInstance(id());
117116

118-
Command command(instance.crosvm_binary());
119-
command.AddParameter("powerbtn");
120-
command.AddParameter(instance.CrosvmSocketPath());
117+
Command command = Command(instance.crosvm_binary())
118+
.AddParameter("powerbtn")
119+
.AddParameter(instance.CrosvmSocketPath());
121120

122121
LOG(INFO) << "Pressing power button";
123-
std::string output;
124-
std::string error;
125-
auto ret = RunWithManagedStdio(std::move(command), NULL, &output, &error);
126-
CF_EXPECT_EQ(ret, 0,
127-
"crosvm powerbtn returned: " << ret << "\n"
128-
<< output << "\n"
129-
<< error);
122+
CF_EXPECT(RunAndCaptureStdout(std::move(command)));
130123
return {};
131124
}
132125

0 commit comments

Comments
 (0)