commit c92ca71ee63fc3470c1b4810d9a0af1ea5300645
author Jack Rosenthal <jrosenth@chromium.org> Mon Dec 09 10:39:09 2019 -0700
committer Commit Bot <commit-bot@chromium.org> Tue Dec 10 06:34:12 2019 +0000
tree 9eca25d0edb8a687dbf31579773037ac15d39266
parent 4a106ceaadb71cf33173b226652c4ffdac921d2d

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 <jrosenth@chromium.org>
Reviewed-on: https://chromium-review.googlesource.com/c/chromiumos/platform/vboot_reference/+/1957760
Tested-by: Julius Werner <jwerner@chromium.org>
Reviewed-by: Julius Werner <jwerner@chromium.org>

host/lib/include/subprocess.h

diff --git a/host/lib/include/subprocess.h b/host/lib/include/subprocess.h
index cddcf05..61a214a 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 @@
 			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 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.
  *
- * If either input, output, or error are set to NULL, the will be
+ * @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, 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,

host/lib/subprocess.c

diff --git a/host/lib/subprocess.c b/host/lib/subprocess.c
index 5721a57..95a6e4d 100644
--- a/host/lib/subprocess.c
+++ b/host/lib/subprocess.c
@@ -74,6 +74,8 @@
 			return -1;
 		}
 		break;
+	default:
+		return -1;
 	}
 
 	return dup2(target_fd, fd);
@@ -110,7 +112,7 @@
 		bytes_to_write -= write_rv;
 	}
 
-cleanup:
+ cleanup:
 	close(target->buffer._pipefd[1]);
 	return rv;
 }
@@ -154,7 +156,7 @@
 	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 @@
 	if (WIFEXITED(status))
 		return WEXITSTATUS(status);
 
-fail:
+ fail:
 	if (program_name)
 		perror(program_name);
 	else

tests/subprocess_tests.c

diff --git a/tests/subprocess_tests.c b/tests/subprocess_tests.c
index 138c101..58c8f23 100644
--- a/tests/subprocess_tests.c
+++ b/tests/subprocess_tests.c
@@ -179,7 +179,5 @@
 	test_subprocess_small_output_buffer();
 	test_subprocess_return_code_failure();
 
-	if (!gTestSuccess)
-		return 255;
-	return 0;
+	return gTestSuccess ? 0 : 255;
 }