Re: [PATCH] commands: go: support passing integer arguments instead of argc/argv

[Sascha Hauer <sha@pengutronix.de>]

On Fri, Jul 21, 2023 at 01:10:07PM +0200, Ahmad Fatoum wrote:
> The go command accepts extra arguments, which are passed through to the
> application using two parameters: argc to hold the number of arguments
> and argv as a NULL pointer terminated array of pointers to
> nul-terminated strings.
> 
> As the go command is meant to invoke bare metal applications, it is
> useful to be able to pass arguments parsed to integers using the
> default calling convention, so add a new -i argument. The old behavior
> remains the default and a new -s arguments makes it explicit for
> symmetry.
> 
> Signed-off-by: Ahmad Fatoum <a.fatoum@pengutronix.de>
> ---
>  commands/Kconfig |  5 ++++-
>  commands/go.c    | 54 +++++++++++++++++++++++++++++++++++++++++-------
>  2 files changed, 51 insertions(+), 8 deletions(-)
> 
> diff --git a/commands/Kconfig b/commands/Kconfig
> index 7ca7b56fa54e..987cc286fc73 100644
> --- a/commands/Kconfig
> +++ b/commands/Kconfig
> @@ -461,12 +461,15 @@ config CMD_GO
>  	help
>  	  Start application at address or file
>  
> -	  Usage: go ADDR [ARG...]
> +	  Usage: go [-si] ADDR [ARG...]
>  
>  	  Start application at ADDR passing ARG as arguments.
>  
>  	  If addr does not start with a digit it is interpreted as a filename
>  	  in which case the file is memmapped and executed
> +	  Options:
> +		  -s	pass all arguments as strings referenced by argc, argv arguments (default)
> +		  -i	pass up to four integer arguments using default calling convention
>  
>  config CMD_LOADB
>  	depends on CONSOLE_FULL
> diff --git a/commands/go.c b/commands/go.c
> index 0c0b4cb15e4d..c0c0f5a0c562 100644
> --- a/commands/go.c
> +++ b/commands/go.c
> @@ -8,21 +8,44 @@
>  #include <complete.h>
>  #include <fs.h>
>  #include <fcntl.h>
> +#include <getopt.h>
>  #include <linux/ctype.h>
>  #include <errno.h>
>  
> +#define INT_ARGS_MAX	4
> +
>  static int do_go(int argc, char *argv[])
>  {
>  	void	*addr;
>  	int     rcode = 1;
>  	int	fd = -1;
> -	int	(*func)(int argc, char *argv[]);
> +	void	(*func)(ulong r0, ulong r1, ulong r2, ulong r3);
> +	int	opt;
> +	ulong	arg[INT_ARGS_MAX] = {};
> +	bool	pass_argv = true;
>  
> -	if (argc < 2)
> +	while ((opt = getopt(argc, argv, "+si")) > 0) {
> +		switch (opt) {
> +		case 's':
> +			pass_argv = true;
> +			break;
> +		case 'i':
> +			pass_argv = false;
> +			break;
> +		default:
> +			return COMMAND_ERROR_USAGE;
> +		}
> +	}
> +
> +	/* skip options */
> +	argv += optind;
> +	argc -= optind;
> +
> +	if (argc == 0 || (!pass_argv && argc - 1 > INT_ARGS_MAX))
>  		return COMMAND_ERROR_USAGE;
>  
> -	if (!isdigit(*argv[1])) {
> -		fd = open(argv[1], O_RDONLY);
> +	if (!isdigit(*argv[0])) {
> +		fd = open(argv[0], O_RDONLY);
>  		if (fd < 0) {
>  			perror("open");
>  			goto out;
> @@ -34,7 +57,7 @@ static int do_go(int argc, char *argv[])
>  			goto out;
>  		}
>  	} else
> -		addr = (void *)simple_strtoul(argv[1], NULL, 16);
> +		addr = (void *)simple_strtoul(argv[0], NULL, 16);
>  
>  	printf("## Starting application at 0x%p ...\n", addr);
>  
> @@ -44,7 +67,21 @@ static int do_go(int argc, char *argv[])
>  
>  	shutdown_barebox();
>  
> -	func(argc - 1, &argv[1]);
> +	if (pass_argv) {
> +		arg[0] = argc;
> +		arg[1] = (ulong)argv;
> +	} else {
> +		int i;
> +
> +		/* skip argv[0] */
> +		argc--;
> +		argv++;
> +
> +		for (i = 0; i < argc; i++)
> +			kstrtoul(argv[i], 0, &arg[i]);

kstrtoul returns error codes and here they could be used to catch some
misusages of the command like accidently passing string arguments
together with the -i option. You should move the call to
shutdown_barebox() further down and catch these errors.

Sascha

-- 
Pengutronix e.K.                           |                             |
Steuerwalder Str. 21                       | http://www.pengutronix.de/  |
31137 Hildesheim, Germany                  | Phone: +49-5121-206917-0    |
Amtsgericht Hildesheim, HRA 2686           | Fax:   +49-5121-206917-5555 |