From c92ca71ee63fc3470c1b4810d9a0af1ea5300645 Mon Sep 17 00:00:00 2001 From: Jack Rosenthal Date: Mon, 9 Dec 2019 10:39:09 -0700 Subject: lib/subprocess: style updates Just resolving some style comments left on CL:1955805. The CL merged by CQ before I noticed the comments. BUG=none BRANCH=none TEST=compiles Change-Id: I286343e3ee2ecb4cb6092ca99fa46c4a80442e03 Signed-off-by: Jack Rosenthal Reviewed-on: https://chromium-review.googlesource.com/c/chromiumos/platform/vboot_reference/+/1957760 Tested-by: Julius Werner Reviewed-by: Julius Werner --- host/lib/include/subprocess.h | 35 +++++++++++++++++++---------------- host/lib/subprocess.c | 8 +++++--- tests/subprocess_tests.c | 4 +--- 3 files changed, 25 insertions(+), 22 deletions(-) diff --git a/host/lib/include/subprocess.h b/host/lib/include/subprocess.h index cddcf05c..61a214a2 100644 --- a/host/lib/include/subprocess.h +++ b/host/lib/include/subprocess.h @@ -1,10 +1,10 @@ /* Copyright 2019 The Chromium OS Authors. All rights reserved. * Use of this source code is governed by a BSD-style license that can be * found in the LICENSE file. + * + * Library for creating subprocesses in a high level manner. */ -/* Library for creating subprocesses in a high level manner. */ - #ifndef VBOOT_REFERENCE_SUBPROCESS_H_ #define VBOOT_REFERENCE_SUBPROCESS_H_ @@ -52,12 +52,14 @@ struct subprocess_target { char *buf; size_t size; - /* This variable is used internally by "run" and - * shouldn't be operated on by the caller. + /* + * This variable is used internally by subprocess_run + * and shouldn't be operated on by the caller. */ int _pipefd[2]; - /* This variable is the output of the number of bytes + /* + * This variable is the output of the number of bytes * read or written. It should be read by the caller, not * set. */ @@ -72,33 +74,34 @@ struct subprocess_target { struct subprocess_target subprocess_null; /** - * A convenience subprocess target which uses TARGET_FD to - * STDIN_FILENO. + * A convenience subprocess target which uses TARGET_FD to STDIN_FILENO. */ struct subprocess_target subprocess_stdin; /** - * A convenience subprocess target which uses TARGET_FD to - * STDOUT_FILENO. + * A convenience subprocess target which uses TARGET_FD to STDOUT_FILENO. */ struct subprocess_target subprocess_stdout; /** - * A convenience subprocess target which uses TARGET_FD to - * STDERR_FILENO. + * A convenience subprocess target which uses TARGET_FD to STDERR_FILENO. */ struct subprocess_target subprocess_stderr; /** - * Call a process described by argv and run until completion. Provide - * input from the subprocess target input, output to the subprocess - * target output, and error to the subprocess target error. + * Call a process and run until completion. + * + * @param argv A NULL-terminated list of arguments describing + * the program to run. + * @param input The subprocess_target connected to stdin. + * @param output The subprocess_target connected to stdout. + * @param error The subprocess_target connected to stderr. * - * If either input, output, or error are set to NULL, the will be + * If either input, output, or error are set to NULL, they will be * &subprocess_stdin, &subprocess_stdout, or &subprocess_stderr * respectively. * - * Returns the exit status on success, or negative values on error. + * @return The exit status on success, or negative values on error. */ int subprocess_run(const char *const argv[], struct subprocess_target *input, diff --git a/host/lib/subprocess.c b/host/lib/subprocess.c index 5721a576..95a6e4d2 100644 --- a/host/lib/subprocess.c +++ b/host/lib/subprocess.c @@ -74,6 +74,8 @@ static int connect_process_target(struct subprocess_target *target, int fd) return -1; } break; + default: + return -1; } return dup2(target_fd, fd); @@ -110,7 +112,7 @@ static int process_target_input(struct subprocess_target *target) bytes_to_write -= write_rv; } -cleanup: + cleanup: close(target->buffer._pipefd[1]); return rv; } @@ -154,7 +156,7 @@ static int process_target_output(struct subprocess_target *target) if (target->type == TARGET_BUFFER_NULL_TERMINATED) target->buffer.buf[target->buffer.bytes_consumed] = '\0'; -cleanup: + cleanup: close(target->buffer._pipefd[0]); return rv; } @@ -228,7 +230,7 @@ int subprocess_run(const char *const argv[], if (WIFEXITED(status)) return WEXITSTATUS(status); -fail: + fail: if (program_name) perror(program_name); else diff --git a/tests/subprocess_tests.c b/tests/subprocess_tests.c index 138c1019..58c8f235 100644 --- a/tests/subprocess_tests.c +++ b/tests/subprocess_tests.c @@ -179,7 +179,5 @@ int main(int argc, char *argv[]) test_subprocess_small_output_buffer(); test_subprocess_return_code_failure(); - if (!gTestSuccess) - return 255; - return 0; + return gTestSuccess ? 0 : 255; } -- cgit v1.2.1