Return a Result<siginfo_t> from Subprocess::Wait

base/cvd/cuttlefish/common/libs/utils/BUILD.bazel

@@ -345,6 +345,7 @@ cf_cc_library(
         "//cuttlefish/common/libs/fs",
         "//cuttlefish/common/libs/utils:contains",
         "//cuttlefish/common/libs/utils:files",
+        "//cuttlefish/posix:strerror",
         "//cuttlefish/result",
         "//libbase",
         "@abseil-cpp//absl/log",

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

@@ -52,6 +52,7 @@
 
 #include "cuttlefish/common/libs/utils/contains.h"
 #include "cuttlefish/common/libs/utils/files.h"
+#include "cuttlefish/posix/strerror.h"
 
 extern char** environ;
 
@@ -207,22 +208,19 @@ int Subprocess::Wait() {
   }
   return retval;
 }
-int Subprocess::Wait(siginfo_t* infop, int options) {
-  if (pid_ < 0) {
-    LOG(ERROR)
-        << "Attempt to wait on invalid pid(has it been waited on already?): "
-        << pid_;
-    return -1;
-  }
-  *infop = {};
-  auto retval = TEMP_FAILURE_RETRY(waitid(P_PID, pid_, infop, options));
+Result<siginfo_t> Subprocess::Wait(int options) {
+  CF_EXPECT_GE(pid_, 0,
+               "Attempt to wait on invalid pid(has it been waited on already?");
+  siginfo_t infop = {};
+  int retval = TEMP_FAILURE_RETRY(waitid(P_PID, pid_, &infop, options));
   // We don't want to wait twice for the same process
-  bool exited = infop->si_code == CLD_EXITED || infop->si_code == CLD_DUMPED;
+  bool exited = infop.si_code == CLD_EXITED || infop.si_code == CLD_DUMPED;
   bool reaped = !(options & WNOWAIT);
   if (exited && reaped) {
     pid_ = -1;
   }
-  return retval;
+  CF_EXPECT_EQ(retval, 0, StrError(errno));
+  return infop;
 }
 
 static Result<void> SendSignalImpl(const int signal, const pid_t pid,
@@ -581,10 +579,7 @@ Result<siginfo_t> Execute(std::vector<std::string> command,
   Subprocess subprocess = cmd.Start(std::move(subprocess_options));
   CF_EXPECT(subprocess.Started(), "Subprocess failed to start.");
 
-  siginfo_t info;
-  CF_EXPECT_EQ(subprocess.Wait(&info, wait_options), 0);
-
-  return info;
+  return CF_EXPECT(subprocess.Wait(wait_options));
 }
 
 }  // namespace cuttlefish

base/cvd/cuttlefish/common/libs/utils/subprocess.h

@@ -75,7 +75,7 @@ class Subprocess {
   // successfully, non-zero otherwise.
   int Wait();
   // Same as waitid(2)
-  int Wait(siginfo_t* infop, int options);
+  Result<siginfo_t> Wait(int options);
   // Whether the command started successfully. It only says whether the call to
   // fork() succeeded or not, it says nothing about exec or successful
   // completion of the command, that's what Wait is for.

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

@@ -127,8 +127,8 @@ Result<void> CvdDisplayCommandHandler::Handle(const CommandRequest& request) {
   Command command =
       CF_EXPECT(BuildCommand(instance_manager_, request, subcmd_args, env));
 
-  siginfo_t infop;  // NOLINT(misc-include-cleaner)
-  command.Start().Wait(&infop, WEXITED);
+  // NOLINTNEXTLINE(misc-include-cleaner)
+  siginfo_t infop = CF_EXPECT(command.Start().Wait(WEXITED));
 
   CF_EXPECT(CheckProcessExitedNormally(infop));
   return {};

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

@@ -105,8 +105,8 @@ Result<void> CvdEnvCommandHandler::Handle(const CommandRequest& request) {
                         ? CF_EXPECT(HelpCommand(request))
                         : CF_EXPECT(NonHelpCommand(instance_manager_, request));
 
-  siginfo_t infop;  // NOLINT(misc-include-cleaner)
-  command.Start().Wait(&infop, WEXITED);
+  // NOLINTNEXTLINE(misc-include-cleaner)
+  siginfo_t infop = CF_EXPECT(command.Start().Wait(WEXITED));
 
   CF_EXPECT(CheckProcessExitedNormally(infop));
   return {};

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

@@ -93,8 +93,8 @@ Result<void> CvdSnapshotCommandHandler::Handle(const CommandRequest& request) {
   Command command =
       CF_EXPECT(GenerateCommand(request, subcmd, subcmd_args, request.Env()));
 
-  siginfo_t infop;  // NOLINT(misc-include-cleaner)
-  command.Start().Wait(&infop, WEXITED);
+  // NOLINTNEXTLINE(misc-include-cleaner)
+  siginfo_t infop = CF_EXPECT(command.Start().Wait(WEXITED));
 
   CF_EXPECT(CheckProcessExitedNormally(infop));
   return {};

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

@@ -146,8 +146,7 @@ Result<void> LocalInstance::PressPowerBtnLegacy() {
 
   VLOG(0) << "Executing: " << cmd.ToString();
 
-  siginfo_t infop;
-  cmd.Start().Wait(&infop, WEXITED);
+  siginfo_t infop = CF_EXPECT(cmd.Start().Wait(WEXITED));
   CF_EXPECT(CheckProcessExitedNormally(infop));
 
   return {};

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

@@ -55,7 +55,6 @@ cf_cc_library(
     copts = COPTS + ["-Werror=sign-compare"],
     deps = [
         "//cuttlefish/common/libs/utils:subprocess",
-        "//cuttlefish/posix:strerror",
         "//cuttlefish/result",
     ],
 )

base/cvd/cuttlefish/host/commands/cvd/utils/subprocess_waiter.cpp

@@ -19,7 +19,6 @@
 #include <mutex>
 
 #include "cuttlefish/common/libs/utils/subprocess.h"
-#include "cuttlefish/posix/strerror.h"
 #include "cuttlefish/result/result.h"
 
 namespace cuttlefish {
@@ -38,17 +37,13 @@ Result<siginfo_t> SubprocessWaiter::Wait() {
   CF_EXPECT(!interrupted_, "Interrupted");
   CF_EXPECT(subprocess_.has_value());
 
-  siginfo_t infop{};
-
   interrupt_lock.unlock();
 
   // This blocks until the process exits, but doesn't reap it.
-  auto result = subprocess_->Wait(&infop, WEXITED | WNOWAIT);
-  CF_EXPECTF(result != -1, "Lost track of subprocess pid: {}", StrError(errno));
+  siginfo_t infop = CF_EXPECT(subprocess_->Wait(WEXITED | WNOWAIT));
   interrupt_lock.lock();
   // Perform a reaping wait on the process (which should already have exited).
-  result = subprocess_->Wait(&infop, WEXITED);
-  CF_EXPECT(result != -1, "Lost track of subprocess pid");
+  infop = CF_EXPECT(subprocess_->Wait(WEXITED));
   // The double wait avoids a race around the kernel reusing pids. Waiting
   // with WNOWAIT won't cause the child process to be reaped, so the kernel
   // won't reuse the pid until the Wait call below, and any kill signals won't

base/cvd/cuttlefish/host/libs/process_monitor/process_monitor.cc

@@ -159,14 +159,14 @@ Result<void> StopSubprocesses(std::vector<MonitorEntry>& monitored) {
       LOG(WARNING) << "Error in stopping \"" << it.cmd->GetShortName() << "\"";
       return false;
     }
-    siginfo_t infop;
-    auto success = it.proc->Wait(&infop, WEXITED);
-    if (success < 0) {
-      LOG(WARNING) << "Failed to wait for process " << it.cmd->GetShortName();
+    Result<siginfo_t> infop = it.proc->Wait(WEXITED);
+    if (!infop.ok()) {
+      LOG(WARNING) << "Failed to wait for process " << it.cmd->GetShortName()
+                   << ": " << infop.error();
       return false;
     }
     if (stop_result == StopperResult::kStopCrash) {
-      LogSubprocessExit(it.cmd->GetShortName(), infop);
+      LogSubprocessExit(it.cmd->GetShortName(), *infop);
     }
     return true;
   };