Index: base/process/launch_posix.cc |
diff --git a/base/process/launch_posix.cc b/base/process/launch_posix.cc |
index 99582f59cfcfa760b1459b987e962c56d8a76c9d..73ebc73ae60607a6d3885a2441c644999a91bac7 100644 |
--- a/base/process/launch_posix.cc |
+++ b/base/process/launch_posix.cc |
@@ -511,14 +511,6 @@ void RaiseProcessToHighPriority() { |
// setpriority() or sched_getscheduler, but these all require extra rights. |
} |
-// Return value used by GetAppOutputInternal to encapsulate the various exit |
-// scenarios from the function. |
-enum GetAppOutputInternalResult { |
- EXECUTE_FAILURE, |
- EXECUTE_SUCCESS, |
- GOT_MAX_OUTPUT, |
-}; |
- |
// Executes the application specified by |argv| and wait for it to exit. Stores |
// the output (stdout) in |output|. If |do_search_path| is set, it searches the |
// path for the application; in that case, |envp| must be null, and it will use |
@@ -526,21 +518,14 @@ enum GetAppOutputInternalResult { |
// specify the path of the application, and |envp| will be used as the |
// environment. If |include_stderr| is true, includes stderr otherwise redirects |
// it to /dev/null. |
-// If we successfully start the application and get all requested output, we |
-// return GOT_MAX_OUTPUT, or if there is a problem starting or exiting |
-// the application we return RUN_FAILURE. Otherwise we return EXECUTE_SUCCESS. |
-// The GOT_MAX_OUTPUT return value exists so a caller that asks for limited |
-// output can treat this as a success, despite having an exit code of SIG_PIPE |
-// due to us closing the output pipe. |
-// In the case of EXECUTE_SUCCESS, the application exit code will be returned |
-// in |*exit_code|, which should be checked to determine if the application |
-// ran successfully. |
-static GetAppOutputInternalResult GetAppOutputInternal( |
+// The return value of the function indicates success or failure. In the case of |
+// success, the application exit code will be returned in |*exit_code|, which |
+// should be checked to determine if the application ran successfully. |
+static bool GetAppOutputInternal( |
const std::vector<std::string>& argv, |
char* const envp[], |
bool include_stderr, |
std::string* output, |
- size_t max_output, |
bool do_search_path, |
int* exit_code) { |
// Doing a blocking wait for another command to finish counts as IO. |
@@ -562,13 +547,13 @@ static GetAppOutputInternalResult GetAppOutputInternal( |
DCHECK(!do_search_path ^ !envp); |
if (pipe(pipe_fd) < 0) |
- return EXECUTE_FAILURE; |
+ return false; |
switch (pid = fork()) { |
case -1: // error |
close(pipe_fd[0]); |
close(pipe_fd[1]); |
- return EXECUTE_FAILURE; |
+ return false; |
case 0: // child |
{ |
// DANGER: no calls to malloc or locks are allowed from now on: |
@@ -621,17 +606,13 @@ static GetAppOutputInternalResult GetAppOutputInternal( |
output->clear(); |
char buffer[256]; |
- size_t output_buf_left = max_output; |
- ssize_t bytes_read = 1; // A lie to properly handle |max_output == 0| |
- // case in the logic below. |
+ ssize_t bytes_read = 0; |
Lei Zhang
2016/07/04 20:33:04
This can be declared in the while loop now.
benwells
2016/07/05 03:48:38
Done.
|
- while (output_buf_left > 0) { |
- bytes_read = HANDLE_EINTR(read(pipe_fd[0], buffer, |
- std::min(output_buf_left, sizeof(buffer)))); |
+ while (true) { |
+ bytes_read = HANDLE_EINTR(read(pipe_fd[0], buffer, sizeof(buffer))); |
if (bytes_read <= 0) |
break; |
output->append(buffer, bytes_read); |
- output_buf_left -= static_cast<size_t>(bytes_read); |
} |
close(pipe_fd[0]); |
@@ -640,13 +621,9 @@ static GetAppOutputInternalResult GetAppOutputInternal( |
Process process(pid); |
bool success = process.WaitForExit(exit_code); |
Lei Zhang
2016/07/04 20:33:04
No need for a success variable. Just return.
benwells
2016/07/05 03:48:38
Done.
|
- // If we stopped because we read as much as we wanted, we return |
- // GOT_MAX_OUTPUT (because the child may exit due to |SIGPIPE|). |
- if (!output_buf_left && bytes_read > 0) |
- return GOT_MAX_OUTPUT; |
- else if (success) |
- return EXECUTE_SUCCESS; |
- return EXECUTE_FAILURE; |
+ if (success) |
+ return true; |
+ return false; |
} |
} |
} |
@@ -658,42 +635,24 @@ bool GetAppOutput(const CommandLine& cl, std::string* output) { |
bool GetAppOutput(const std::vector<std::string>& argv, std::string* output) { |
// Run |execve()| with the current environment and store "unlimited" data. |
int exit_code; |
- GetAppOutputInternalResult result = GetAppOutputInternal( |
- argv, NULL, false, output, std::numeric_limits<std::size_t>::max(), true, |
- &exit_code); |
- return result == EXECUTE_SUCCESS && exit_code == EXIT_SUCCESS; |
+ bool result = |
+ GetAppOutputInternal(argv, NULL, false, output, true, &exit_code); |
Lei Zhang
2016/07/04 20:33:04
Might as well s/NULL/nullptr/ while we are here.
benwells
2016/07/05 03:48:38
Done.
|
+ return result && exit_code == EXIT_SUCCESS; |
} |
bool GetAppOutputAndError(const CommandLine& cl, std::string* output) { |
// Run |execve()| with the current environment and store "unlimited" data. |
int exit_code; |
- GetAppOutputInternalResult result = GetAppOutputInternal( |
- cl.argv(), NULL, true, output, std::numeric_limits<std::size_t>::max(), |
- true, &exit_code); |
- return result == EXECUTE_SUCCESS && exit_code == EXIT_SUCCESS; |
-} |
- |
-// TODO(viettrungluu): Conceivably, we should have a timeout as well, so we |
-// don't hang if what we're calling hangs. |
-bool GetAppOutputRestricted(const CommandLine& cl, |
- std::string* output, size_t max_output) { |
- // Run |execve()| with the empty environment. |
- char* const empty_environ = NULL; |
- int exit_code; |
- GetAppOutputInternalResult result = GetAppOutputInternal( |
- cl.argv(), &empty_environ, false, output, max_output, false, &exit_code); |
- return result == GOT_MAX_OUTPUT || (result == EXECUTE_SUCCESS && |
- exit_code == EXIT_SUCCESS); |
+ bool result = |
+ GetAppOutputInternal(cl.argv(), NULL, true, output, true, &exit_code); |
+ return result && exit_code == EXIT_SUCCESS; |
} |
bool GetAppOutputWithExitCode(const CommandLine& cl, |
std::string* output, |
int* exit_code) { |
// Run |execve()| with the current environment and store "unlimited" data. |
- GetAppOutputInternalResult result = GetAppOutputInternal( |
- cl.argv(), NULL, false, output, std::numeric_limits<std::size_t>::max(), |
- true, exit_code); |
- return result == EXECUTE_SUCCESS; |
+ return GetAppOutputInternal(cl.argv(), NULL, false, output, true, exit_code); |
} |
#endif // !defined(OS_NACL_NONSFI) |