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 <[email protected]> --- 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
