[PATCH v4 03/14] lib: getopt: Drop printed error messages and the silent variant

[Simon Glass]

Inside __getopt() two printf() calls report 'invalid option' and
'option requires an argument'. Every U-Boot caller that uses getopt()
already detects '?' and ':' return values and prints its own usage
via CMD_RET_USAGE, so the messages are redundant. They also bring
in two format strings and the printf() argument plumbing.

Drop both printf() calls and the 'silent' that gates them. Rename
__getopt() to getopt() (the function is now small enough not to need an
inline trampoline), and turn getopt_silent() into a static-inline alias
of getopt() so the existing unit tests keep working without change.

Update the bdinfo help test, which expected getopt() to print
'bdinfo: invalid option -- h' for an unknown option; with the messages
gone, 'bdinfo -h' falls straight through to the usage text.

This saves around 120 bytes on 64-bit archs.

Signed-off-by: Simon Glass <sjg at chromium.org>
---

Changes in v4:
- Reword the commit message a little
- Fix the getopt_silent() kernel-doc and the bdinfo help test

 include/getopt.h  | 29 ++++++++++++++++-------------
 lib/getopt.c      |  6 +-----
 test/cmd/bdinfo.c |  1 -
 3 files changed, 17 insertions(+), 19 deletions(-)

diff --git a/include/getopt.h b/include/getopt.h
index 03db6647cc6..629c2e7d85c 100644
--- a/include/getopt.h
+++ b/include/getopt.h
@@ -25,8 +25,8 @@ struct getopt_state {
 	char *argv[CONFIG_SYS_MAXARGS + 1];
 #else
 	/**
-	 * @argv: Borrowed pointer to the caller's argv. In POSIX mode
-	 * getopt() never reorders, so no writable copy is needed.
+	 * @argv: Borrowed pointer to the caller's argv. In POSIX mode getopt()
+	 * never reorders, so no writable copy is needed.
 	 */
 	char *const *argv;
 #endif
@@ -71,8 +71,6 @@ struct getopt_state {
 void getopt_init_state(struct getopt_state *gs, int argc,
 		       char *const argv[]);
 
-int __getopt(struct getopt_state *gs, const char *optstring, bool silent);
-
 /**
  * getopt() - Parse short command-line options
  * @gs: Internal state and out-of-band return arguments. This must be
@@ -99,6 +97,10 @@ int __getopt(struct getopt_state *gs, const char *optstring, bool silent);
  * getopt() instead stops at the first non-option (POSIX ``getopt``
  * behaviour). An ``--`` argument terminates option scanning in either mode.
  *
+ * getopt() does not print any error messages; the caller is expected to
+ * detect '?' and ':' and report via its own usage string (typically
+ * CMD_RET_USAGE).
+ *
  * An example invocation of getopt() might look like::
  *
  *     char *argv[] = { "program", "-cbx", "-a", "foo", "bar", 0 };
@@ -132,21 +134,22 @@ int __getopt(struct getopt_state *gs, const char *optstring, bool silent);
  * @gs.index is always set to the index of the next unparsed argument in
  * @gs.argv.
  */
-static inline int getopt(struct getopt_state *gs, const char *optstring)
-{
-	return __getopt(gs, optstring, false);
-}
+int getopt(struct getopt_state *gs, const char *optstring);
 
 /**
- * getopt_silent() - Parse short command-line options silently
- * @gs: State
- * @optstring: Option specification
+ * getopt_silent() - Compatibility alias for getopt()
+ * @gs: getopt state
+ * @optstring: option specification, as for getopt()
+ *
+ * getopt() no longer prints error messages, so this is identical to
+ * getopt(). Kept for callers (notably the unit tests) that named it
+ * explicitly.
  *
- * Same as getopt(), except no error messages are printed.
+ * Return: same as getopt()
  */
 static inline int getopt_silent(struct getopt_state *gs, const char *optstring)
 {
-	return __getopt(gs, optstring, true);
+	return getopt(gs, optstring);
 }
 
 /**
diff --git a/lib/getopt.c b/lib/getopt.c
index 1b3dfa5f949..3b1dc7773ec 100644
--- a/lib/getopt.c
+++ b/lib/getopt.c
@@ -39,7 +39,7 @@ void getopt_init_state(struct getopt_state *gs, int argc, char *const argv[])
 	gs->arg_index = 1;
 }
 
-int __getopt(struct getopt_state *gs, const char *optstring, bool silent)
+int getopt(struct getopt_state *gs, const char *optstring)
 {
 	char curopt;	/* current option character */
 	const char *curoptp;	/* pointer to the current option in optstring */
@@ -117,8 +117,6 @@ int __getopt(struct getopt_state *gs, const char *optstring, bool silent)
 	curoptp = strchr(optstring, curopt);
 
 	if (!curoptp) {
-		if (!silent)
-			printf("%s: invalid option -- %c\n", argv[0], curopt);
 		gs->opt = curopt;
 		gs->arg_index++;
 		return '?';
@@ -173,8 +171,6 @@ int __getopt(struct getopt_state *gs, const char *optstring, bool silent)
 	gs->arg_index = 1;
 
 	if (gs->index + NONOPTS(gs) >= argc || argv[gs->index][0] == '-') {
-		if (!silent)
-			printf("option requires an argument -- %c\n", curopt);
 		gs->opt = curopt;
 		return ':';
 	}
diff --git a/test/cmd/bdinfo.c b/test/cmd/bdinfo.c
index 7b7fb0894dd..195a86b8216 100644
--- a/test/cmd/bdinfo.c
+++ b/test/cmd/bdinfo.c
@@ -304,7 +304,6 @@ static int bdinfo_test_help(struct unit_test_state *uts)
 		ut_assertok(bdinfo_test_all(uts));
 	} else {
 		ut_asserteq(1, run_commandf("bdinfo -h"));
-		ut_assert_nextlinen("bdinfo: invalid option -- h");
 		ut_assert_nextlinen("bdinfo - print Board Info structure");
 		ut_assert_nextline_empty();
 		ut_assert_nextlinen("Usage:");
-- 
2.43.0