shill: Set pid in ProcessManagerTest Currently, in ProcessManagerTest.StartProcessWithMinijailWithStdout, we don't update the pid pointer when handling RunEnvPipesAndDestroy(), and thus the local variable |pid| in ProcessManager::StartProcessInMinijailWithPipesInternal() is uninitialized. Sometimes it will have value -1 and we treat this value as a signal of failure so the unit test will fail. This patch fixes this issue. BUG=b:253349908 TEST=FEATURES="test" emerge-${BOARD} shill Change-Id: I59cdd3243b3559d7e003d0a436f0b69df1e7f8cc Reviewed-on: https://chromium-review.googlesource.com/c/chromiumos/platform2/+/3951750 Reviewed-by: Hugo Benichi <hugobenichi@google.com> Reviewed-by: Taoyu Li <taoyl@chromium.org> Commit-Queue: Jie Jiang <jiejiang@chromium.org> Tested-by: Jie Jiang <jiejiang@chromium.org>

diff --git a/shill/process_manager.cc b/shill/process_manager.cc
index 28cc6a5..e5e8a99 100644
--- a/shill/process_manager.cc
+++ b/shill/process_manager.cc

@@ -108,7 +108,7 @@
       base::BindOnce(&SetupChild, environment, terminate_with_parent));
   if (!process->Start()) {
     LOG(ERROR) << "Failed to start child process for " << program.value();
-    return -1;
+    return kInvalidPID;
   }
 
   // Setup watcher for the child process.
@@ -136,7 +136,7 @@
     struct std_file_descriptors std_fds) {
   pid_t pid = StartProcessInMinijailWithPipesInternal(
       spawn_source, program, arguments, environment, minijail_options, std_fds);
-  if (pid == -1) {
+  if (pid == kInvalidPID) {
     return pid;
   }
   watched_processes_[pid].exit_callback = std::move(exit_callback);
@@ -154,7 +154,7 @@
   pid_t pid = StartProcessInMinijailWithPipesInternal(
       spawn_source, program, arguments, environment, minijail_options,
       {.stdout_fd = &stdout_fd});
-  if (pid == -1) {
+  if (pid == kInvalidPID) {
     return pid;
   }
   auto& watched_process = watched_processes_[pid];
@@ -322,7 +322,7 @@
   if (!minijail_->DropRoot(jail, minijail_options.user.c_str(),
                            minijail_options.group.c_str())) {
     LOG(ERROR) << "Minijail failed to drop root privileges?";
-    return -1;
+    return kInvalidPID;
   }
 
   if (minijail_options.inherit_supplementary_groups) {
@@ -349,7 +349,7 @@
   if (!minijail_->RunEnvPipesAndDestroy(jail, args, env, &pid, std_fds.stdin_fd,
                                         std_fds.stdout_fd, std_fds.stderr_fd)) {
     LOG(ERROR) << "Unable to spawn " << program.value() << " in a jail.";
-    return -1;
+    return kInvalidPID;
   }
 
   CHECK(process_reaper_.WatchForChild(

diff --git a/shill/process_manager.h b/shill/process_manager.h
index 1cb71e0..6949a4b 100644
--- a/shill/process_manager.h
+++ b/shill/process_manager.h

@@ -66,6 +66,7 @@
   };
 
   static constexpr base::TimeDelta kTerminationTimeout = base::Seconds(2);
+  static constexpr pid_t kInvalidPID = -1;
 
   virtual ~ProcessManager();
 
@@ -80,10 +81,10 @@
 
   // Create and start a process for |program| with |arguments|. |environment|
   // variables will be setup in the child process before exec the |program|.
-  // |terminate_with_parent| is used to indicate if child process should
-  // self terminate if the parent process exits.  |exit_callback| will be
-  // invoked when child process exits (not terminated by us).  Return -1
-  // if failed to start the process, otherwise, return the pid of the child
+  // |terminate_with_parent| is used to indicate if child process should self
+  // terminate if the parent process exits.  |exit_callback| will be invoked
+  // when child process exits (not terminated by us).  Return kInvalidPID if
+  // failed to start the process, otherwise, return the pid of the child
   // process.
   virtual pid_t StartProcess(
       const base::Location& spawn_source,

diff --git a/shill/process_manager_test.cc b/shill/process_manager_test.cc
index 9fb90f3..f7c520e 100644
--- a/shill/process_manager_test.cc
+++ b/shill/process_manager_test.cc

@@ -27,7 +27,7 @@
 using testing::Return;
 using testing::SetArgPointee;
 using testing::StrEq;
-using testing::WithArg;
+using testing::WithArgs;
 
 namespace shill {
 
@@ -285,6 +285,7 @@
 }
 
 TEST_F(ProcessManagerTest, StartProcessWithMinijailWithStdout) {
+  const pid_t kPid = 456;
   const std::string kProgram = "/usr/bin/dump";
   const std::vector<std::string> kArgs = {"-b", "-g"};
   const std::map<std::string, std::string> kEnv = {
@@ -318,16 +319,19 @@
   EXPECT_CALL(minijail_,
               RunEnvPipesAndDestroy(_, IsProcessArgs(kProgram, kArgs),
                                     IsProcessEnv(kEnv), _, _, _, _))
-      .WillOnce(WithArg<5>([&process_side_stdout_fd](int* stdout_fd) {
-        int fds[2];
-        CHECK(base::CreateLocalNonBlockingPipe(fds));
-        *stdout_fd = fds[0];
-        process_side_stdout_fd.reset(fds[1]);
-        return true;
-      }));
+      .WillOnce(
+          WithArgs<3, 5>([&process_side_stdout_fd](pid_t* pid, int* stdout_fd) {
+            int fds[2];
+            CHECK(base::CreateLocalNonBlockingPipe(fds));
+            *stdout_fd = fds[0];
+            process_side_stdout_fd.reset(fds[1]);
+            *pid = kPid;
+            return true;
+          }));
   pid_t actual_pid = process_manager_->StartProcessInMinijailWithStdout(
       FROM_HERE, base::FilePath(kProgram), kArgs, kEnv, minijail_options,
       base::BindLambdaForTesting(callback));
+  ASSERT_NE(actual_pid, ProcessManager::kInvalidPID);
 
   constexpr int kExpectedExitStatus = 123;
   std::string expected_stdout;