Re: [RFC PATCH 07/11] cmd: echo: Use getopt() with '+' prefix for option parsing

[Sean Anderson <seanga2@gmail.com>]

On 5/15/26 16:32, Simon Glass wrote:
> The 'echo' command's option parser is a single strcmp against argv[1]
> that decides whether to suppress the trailing newline. Convert it to
> getopt() so echo follows the same shape as the other commands in this
> series and exercises the '+' prefix that POSIX-style 'stop at first
> non-option' callers need.
> 
> The optstring uses the '+' prefix to preserve bash echo behaviour: -n is
> honoured only as the very first argument, so 'echo hello -n' still
> prints 'hello -n\n' verbatim.
> 
> Two minor differences from the bash builtin remain, both of which
> the user can work around with quoting or --:
> 
> * -x (an unknown short option) returns CMD_RET_USAGE rather than
>    printing literally; use 'echo -- -x' to print it.
> * -nfoo (joined form) parses -n and then errors on the trailing
>    characters.

Why? This introduces incompatibility with echo as it exists today as
well as unix-style echo. We can have an (almost) completely-compatible
echo with

	getopt_init_state(&gs, argc, argv);
	while (getopt_silent(&gs, "+n") == 'n')
		newline = false;

	for (i = gs.index; i < argc; ++i) {
		<snip>

The only difference is that something like "echo -na" will result in
"-na" and not "-na\n". IMO echo is not a good candidate for getopt
due to its unusual argument handling. Even coreutils echo does not
use getopt. So I think we should really leave echo as-is.

> Add three test cases that pin down the new behaviour: trailing -n stays
> literal, -- ends option parsing, and -- is consumed even after a
> recognised flag. The positional loop uses getopt_pop() as an iterator.
> CMD_ECHO selects GETOPT so the parser is linked in on boards that don't
> already enable it.
> 
> Signed-off-by: Simon Glass <sjg@chromium.org>
> ---
> 
>   cmd/Kconfig          |  1 +
>   cmd/echo.c           | 23 ++++++++++++++---------
>   test/cmd/test_echo.c | 10 ++++++++++
>   3 files changed, 25 insertions(+), 9 deletions(-)
> 
> diff --git a/cmd/Kconfig b/cmd/Kconfig
> index c71c6824a19..709696c3c41 100644
> --- a/cmd/Kconfig
> +++ b/cmd/Kconfig
> @@ -1916,6 +1916,7 @@ config CMD_CAT
>   config CMD_ECHO
>   	bool "echo"
>   	default y
> +	select GETOPT
>   	help
>   	  Echo args to console
>   
> diff --git a/cmd/echo.c b/cmd/echo.c
> index d1346504cfb..63422c75cc6 100644
> --- a/cmd/echo.c
> +++ b/cmd/echo.c
> @@ -5,27 +5,32 @@
>    */
>   
>   #include <command.h>
> -#include <linux/string.h>
> +#include <getopt.h>
>   
>   static int do_echo(struct cmd_tbl *cmdtp, int flag, int argc,
>   		   char *const argv[])
>   {
> -	int i = 1;
> +	struct getopt_state gs;
>   	bool space = false;
>   	bool newline = true;
> +	char *arg;
> +	int opt;
>   
> -	if (argc > 1) {
> -		if (!strcmp(argv[1], "-n")) {
> +	getopt_init_state(&gs, argc, argv);
> +	while ((opt = getopt(&gs, "+n")) > 0) {
> +		switch (opt) {
> +		case 'n':
>   			newline = false;
> -			++i;
> +			break;
> +		default:
> +			return CMD_RET_USAGE;
>   		}
>   	}
>   
> -	for (; i < argc; ++i) {
> -		if (space) {
> +	while ((arg = getopt_pop(&gs))) {
> +		if (space)
>   			putc(' ');
> -		}
> -		puts(argv[i]);
> +		puts(arg);
>   		space = true;
>   	}
>   
> diff --git a/test/cmd/test_echo.c b/test/cmd/test_echo.c
> index 7ed534742f7..5fc139fbe68 100644
> --- a/test/cmd/test_echo.c
> +++ b/test/cmd/test_echo.c
> @@ -35,6 +35,16 @@ static struct test_data echo_data[] = {
>   	/* Test handling of shell variables. */
>   	{"setenv jQx; for jQx in 1 2 3; do echo -n \"${jQx}, \"; done; echo;",
>   	 "1, 2, 3, "},
> +	/* -n only suppresses the newline when it comes before any
> +	 * positional argument; a trailing -n is just another argument.
> +	 */
> +	{"echo hello -n", "hello -n"},
> +	/* "--" ends option parsing, so a following dash-prefixed token
> +	 * is printed verbatim instead of being rejected.
> +	 */
> +	{"echo -- -x", "-x"},
> +	/* "--" is consumed even when it follows a recognised flag. */
> +	{"echo -n -- foo; echo done", "foodone"},
>   };
>   
>   static int lib_test_hush_echo(struct unit_test_state *uts)