[PATCH v2 0/4] cli: run_commandf() coverage and small fixups

[PATCH v2 0/4] cli: run_commandf() coverage and small fixups

[Evgeny Bachinin]

Our company has been using custom variadic version of run_command()
since the beginning of the 2022Y. Recently, we had started upstreaming
activity and figured out, that similar functionality already exists.
Thanks, it's very helpful, because our code uses such variadic API.

Let me share test-cases for run_commandf() to improve coverage and
btw provide several fixups for run_commandf(), found during applying
this API to our custom sources.

Patchset has been tested on our set of devices and sandbox64
(appropriate unit-tests were checked: ut_cmd, ut fdt, ut exit).
Additionally, github CI loop has been passed successfully without
regression in context of github PR [1].

Changes v2 since v1 at [2]:
- rebase
- run_commandf: apply comments
- command_ut: re-write truncation test-case
- test/cmd/fdt.c: fix compilation after rebase

Links:
[1] https://github.com/u-boot/u-boot/pull/277/checks
[2] https://lore.kernel.org/u-boot/20230310185409.22254-1-EABachinin@sberdevices.ru/

Evgeny Bachinin (4):
  cli: run_commandf(): small fixups
  unit-test: cover run_commandf() by test-cases
  test: fdt: fix run_commandf() warnings
  test: exit: fix run_commandf() warnings

 common/cli.c      | 25 +++++++++++++++++++------
 include/command.h | 13 ++++++++++---
 test/cmd/exit.c   | 18 +++++++++---------
 test/cmd/fdt.c    | 16 ++++++++--------
 test/command_ut.c | 34 ++++++++++++++++++++++++++++++++++
 5 files changed, 80 insertions(+), 26 deletions(-)

-- 
2.17.1

[PATCH v2 1/4] cli: run_commandf(): small fixups

[Evgeny Bachinin]

* vsnprintf() can truncate cmd, hence it makes no sense to launch such
command (it's broken). Moreover, it's better to signalize to the caller
about such case (for facilitating debugging or bug hunting).

* Fix kernel-doc warnings:
  include/command.h:264: info: Scanning doc for run_commandf
  include/command.h:268: warning: contents before sections
  include/command.h:271: warning: No description found for return value
                                  of 'run_commandf'

* Add printf-like format attribute to validate at compile-time the format
string against parameters's type.

* Fix compilation error in case of -Wall, -Werror, -Wextra:
error: variable ‘i’ set but not used [-Werror=unused-but-set-variable]

* Drop extra ret variable.

Signed-off-by: Evgeny Bachinin <EABachinin@sberdevices.ru>
---
Changes for v2:
- s/pr_err/pr_debug/ to reduce code size for rare errors
- s/EINVAL/ENOSPC/
- replace on-stack buffer with global console_buffer[]
- use not full (CONFIG_SYS_CBSIZE + 1) space of console_buffer, because
interpreters return no more than CONFIG_SYS_CBSIZE (including \0),
hence we use CONFIG_SYS_CBSIZE as a max command size for run_commandf()
- not apply Reviewed-by due to changes above

 common/cli.c      | 25 +++++++++++++++++++------
 include/command.h | 13 ++++++++++---
 2 files changed, 29 insertions(+), 9 deletions(-)

diff --git a/common/cli.c b/common/cli.c
index 9451e6a142..3916a7b10a 100644
--- a/common/cli.c
+++ b/common/cli.c
@@ -8,6 +8,8 @@
  * JinHua Luo, GuangDong Linux Center, <luo.jinhua@gd-linux.com>
  */
 
+#define pr_fmt(fmt) "cli: %s: " fmt, __func__
+
 #include <common.h>
 #include <bootstage.h>
 #include <cli.h>
@@ -20,6 +22,7 @@
 #include <malloc.h>
 #include <asm/global_data.h>
 #include <dm/ofnode.h>
+#include <linux/errno.h>
 
 #ifdef CONFIG_CMDLINE
 /*
@@ -129,16 +132,26 @@ int run_command_list(const char *cmd, int len, int flag)
 int run_commandf(const char *fmt, ...)
 {
 	va_list args;
-	char cmd[128];
-	int i, ret;
+	int nbytes;
 
 	va_start(args, fmt);
-	i = vsnprintf(cmd, sizeof(cmd), fmt, args);
+	/*
+	 * Limit the console_buffer space being used to CONFIG_SYS_CBSIZE,
+	 * because its last byte is used to fit the replacement of \0 by \n\0
+	 * in underlying hush parser
+	 */
+	nbytes = vsnprintf(console_buffer, CONFIG_SYS_CBSIZE, fmt, args);
 	va_end(args);
 
-	ret = run_command(cmd, 0);
-
-	return ret;
+	if (nbytes < 0) {
+		pr_debug("I/O internal error occurred.\n");
+		return -EIO;
+	} else if (nbytes >= CONFIG_SYS_CBSIZE) {
+		pr_debug("'fmt' size:%d exceeds the limit(%d)\n",
+			 nbytes, CONFIG_SYS_CBSIZE);
+		return -ENOSPC;
+	}
+	return run_command(console_buffer, 0);
 }
 
 /****************************************************************************/
diff --git a/include/command.h b/include/command.h
index 0db4898062..0e153c6046 100644
--- a/include/command.h
+++ b/include/command.h
@@ -13,6 +13,8 @@
 #include <env.h>
 #include <linker_lists.h>
 
+#include <linux/compiler_attributes.h>
+
 #ifndef NULL
 #define NULL	0
 #endif
@@ -260,12 +262,17 @@ int run_command_repeatable(const char *cmd, int flag);
 /**
  * run_commandf() - Run a command created by a format string
  *
- * The command cannot be larger than 127 characters
- *
  * @fmt: printf() format string
  * @...: Arguments to use (flag is always 0)
+ *
+ * The command cannot be larger than (CONFIG_SYS_CBSIZE - 1) characters.
+ *
+ * Return:
+ * Returns 0 on success, -EIO if internal output error occurred, -ENOSPC in
+ *	case of 'fmt' string truncation, or != 0 on error, specific for
+ *	run_command().
  */
-int run_commandf(const char *fmt, ...);
+int run_commandf(const char *fmt, ...) __printf(1, 2);
 
 /**
  * Run a list of commands separated by ; or even \0
-- 
2.17.1

Re: [PATCH v2 1/4] cli: run_commandf(): small fixups

[Simon Glass]

On Mon, 20 Mar 2023 at 21:23, Evgeny Bachinin <EABachinin@sberdevices.ru> wrote:
>
> * vsnprintf() can truncate cmd, hence it makes no sense to launch such
> command (it's broken). Moreover, it's better to signalize to the caller
> about such case (for facilitating debugging or bug hunting).
>
> * Fix kernel-doc warnings:
>   include/command.h:264: info: Scanning doc for run_commandf
>   include/command.h:268: warning: contents before sections
>   include/command.h:271: warning: No description found for return value
>                                   of 'run_commandf'
>
> * Add printf-like format attribute to validate at compile-time the format
> string against parameters's type.
>
> * Fix compilation error in case of -Wall, -Werror, -Wextra:
> error: variable ‘i’ set but not used [-Werror=unused-but-set-variable]
>
> * Drop extra ret variable.
>
> Signed-off-by: Evgeny Bachinin <EABachinin@sberdevices.ru>
> ---
> Changes for v2:
> - s/pr_err/pr_debug/ to reduce code size for rare errors
> - s/EINVAL/ENOSPC/
> - replace on-stack buffer with global console_buffer[]
> - use not full (CONFIG_SYS_CBSIZE + 1) space of console_buffer, because
> interpreters return no more than CONFIG_SYS_CBSIZE (including \0),
> hence we use CONFIG_SYS_CBSIZE as a max command size for run_commandf()
> - not apply Reviewed-by due to changes above
>
>  common/cli.c      | 25 +++++++++++++++++++------
>  include/command.h | 13 ++++++++++---
>  2 files changed, 29 insertions(+), 9 deletions(-)
>

Reviewed-by: Simon Glass <sjg@chromium.org>

Re: [PATCH v2 1/4] cli: run_commandf(): small fixups

[Tom Rini]

[-- Attachment #1: Type: text/plain, Size: 1028 bytes --]

On Mon, Mar 20, 2023 at 11:23:11AM +0300, Evgeny Bachinin wrote:

> * vsnprintf() can truncate cmd, hence it makes no sense to launch such
> command (it's broken). Moreover, it's better to signalize to the caller
> about such case (for facilitating debugging or bug hunting).
> 
> * Fix kernel-doc warnings:
>   include/command.h:264: info: Scanning doc for run_commandf
>   include/command.h:268: warning: contents before sections
>   include/command.h:271: warning: No description found for return value
>                                   of 'run_commandf'
> 
> * Add printf-like format attribute to validate at compile-time the format
> string against parameters's type.
> 
> * Fix compilation error in case of -Wall, -Werror, -Wextra:
> error: variable ‘i’ set but not used [-Werror=unused-but-set-variable]
> 
> * Drop extra ret variable.
> 
> Signed-off-by: Evgeny Bachinin <EABachinin@sberdevices.ru>
> Reviewed-by: Simon Glass <sjg@chromium.org>

Applied to u-boot/next, thanks!

-- 
Tom

[-- Attachment #2: signature.asc --]
[-- Type: application/pgp-signature, Size: 659 bytes --]

[PATCH v2 2/4] unit-test: cover run_commandf() by test-cases

[Evgeny Bachinin]

As run_commandf() is variadic version of run_command() and just a wrapper,
hence apply similar run_command's test-cases.

Let's avoid warning about empty string passing:
warning: zero-length gnu_printf format string [-Wformat-zero-length]
   assert(run_commandf("") == 0);

Signed-off-by: Evgeny Bachinin <EABachinin@sberdevices.ru>
---
Changes for v2:
- s/EINVAL/ENOSPC/
- rewrite test-case for truncation case
- not apply Reviewed-by due to changes above

 test/command_ut.c | 34 ++++++++++++++++++++++++++++++++++
 1 file changed, 34 insertions(+)

diff --git a/test/command_ut.c b/test/command_ut.c
index 9837d10eb5..a74bd109e1 100644
--- a/test/command_ut.c
+++ b/test/command_ut.c
@@ -9,6 +9,8 @@
 #include <command.h>
 #include <env.h>
 #include <log.h>
+#include <string.h>
+#include <linux/errno.h>
 
 static const char test_cmd[] = "setenv list 1\n setenv list ${list}2; "
 		"setenv list ${list}3\0"
@@ -17,6 +19,8 @@ static const char test_cmd[] = "setenv list 1\n setenv list ${list}2; "
 static int do_ut_cmd(struct cmd_tbl *cmdtp, int flag, int argc,
 		     char *const argv[])
 {
+	char long_str[CONFIG_SYS_CBSIZE + 42];
+
 	printf("%s: Testing commands\n", __func__);
 	run_command("env default -f -a", 0);
 
@@ -60,6 +64,36 @@ static int do_ut_cmd(struct cmd_tbl *cmdtp, int flag, int argc,
 
 	assert(run_command("'", 0) == 1);
 
+	/* Variadic function test-cases */
+#pragma GCC diagnostic push
+#pragma GCC diagnostic ignored "-Wformat-zero-length"
+	assert(run_commandf("") == 0);
+#pragma GCC diagnostic pop
+	assert(run_commandf(" ") == 0);
+	assert(run_commandf("'") == 1);
+
+	assert(run_commandf("env %s %s", "delete -f", "list") == 0);
+	/* Expected: "Error: "list" not defined" */
+	assert(run_commandf("printenv list") == 1);
+
+	memset(long_str, 'x', sizeof(long_str));
+	assert(run_commandf("Truncation case: %s", long_str) == -ENOSPC);
+
+	if (IS_ENABLED(CONFIG_HUSH_PARSER)) {
+		assert(run_commandf("env %s %s %s %s", "delete -f", "adder",
+				    "black", "foo") == 0);
+		assert(run_commandf("setenv foo 'setenv %s 1\nsetenv %s 2'",
+				    "black", "adder") == 0);
+		run_command("run foo", 0);
+		assert(env_get("black"));
+		assert(!strcmp("1", env_get("black")));
+		assert(env_get("adder"));
+		assert(!strcmp("2", env_get("adder")));
+	}
+
+	/* Clean up before exit */
+	run_command("env default -f -a", 0);
+
 	printf("%s: Everything went swimmingly\n", __func__);
 	return 0;
 }
-- 
2.17.1

Re: [PATCH v2 2/4] unit-test: cover run_commandf() by test-cases

[Simon Glass]

On Mon, 20 Mar 2023 at 21:23, Evgeny Bachinin <EABachinin@sberdevices.ru> wrote:
>
> As run_commandf() is variadic version of run_command() and just a wrapper,
> hence apply similar run_command's test-cases.
>
> Let's avoid warning about empty string passing:
> warning: zero-length gnu_printf format string [-Wformat-zero-length]
>    assert(run_commandf("") == 0);
>
> Signed-off-by: Evgeny Bachinin <EABachinin@sberdevices.ru>
> ---
> Changes for v2:
> - s/EINVAL/ENOSPC/
> - rewrite test-case for truncation case
> - not apply Reviewed-by due to changes above
>
>  test/command_ut.c | 34 ++++++++++++++++++++++++++++++++++
>  1 file changed, 34 insertions(+)

Reviewed-by: Simon Glass <sjg@chromium.org>

Re: [PATCH v2 2/4] unit-test: cover run_commandf() by test-cases

[Tom Rini]

[-- Attachment #1: Type: text/plain, Size: 519 bytes --]

On Mon, Mar 20, 2023 at 11:23:12AM +0300, Evgeny Bachinin wrote:

> As run_commandf() is variadic version of run_command() and just a wrapper,
> hence apply similar run_command's test-cases.
> 
> Let's avoid warning about empty string passing:
> warning: zero-length gnu_printf format string [-Wformat-zero-length]
>    assert(run_commandf("") == 0);
> 
> Signed-off-by: Evgeny Bachinin <EABachinin@sberdevices.ru>
> Reviewed-by: Simon Glass <sjg@chromium.org>

Applied to u-boot/next, thanks!

-- 
Tom

[-- Attachment #2: signature.asc --]
[-- Type: application/pgp-signature, Size: 659 bytes --]

[PATCH v2 3/4] test: fdt: fix run_commandf() warnings

[Evgeny Bachinin]

Fix warnings both for 32bit and 64bit architecture after adding
printf-like attribute format for run_commandf():
warning: format ‘%x’ expects argument of type ‘unsigned int’, but
  argument 2 has type ‘ulong {aka long unsigned int}’ [-Wformat=]
  ret = run_commandf("fdt addr -c %08x", addr);
                     ^
Signed-off-by: Evgeny Bachinin <EABachinin@sberdevices.ru>
Cc: Marek Vasut <marek.vasut+renesas@mailbox.org>
---
Changes for v2:
- fdt_test_move(): due to addition of __printf(), fix compilation
after rebase atop 79bcd809f49 ("test: cmd: fdt: Test fdt move")
- not apply Reviewed-by due to changes above

 test/cmd/fdt.c | 16 ++++++++--------
 1 file changed, 8 insertions(+), 8 deletions(-)

diff --git a/test/cmd/fdt.c b/test/cmd/fdt.c
index cdbaf8c425..538a30b909 100644
--- a/test/cmd/fdt.c
+++ b/test/cmd/fdt.c
@@ -168,7 +168,7 @@ static int fdt_test_addr(struct unit_test_state *uts)
 	/* Set the working FDT */
 	set_working_fdt_addr(0);
 	ut_assert_nextline("Working FDT set to 0");
-	ut_assertok(run_commandf("fdt addr %08x", addr));
+	ut_assertok(run_commandf("fdt addr %08lx", addr));
 	ut_assert_nextline("Working FDT set to %lx", addr);
 	ut_asserteq(addr, map_to_sysmem(working_fdt));
 	ut_assertok(ut_check_console_end(uts));
@@ -178,7 +178,7 @@ static int fdt_test_addr(struct unit_test_state *uts)
 	/* Set the control FDT */
 	fdt_blob = gd->fdt_blob;
 	gd->fdt_blob = NULL;
-	ret = run_commandf("fdt addr -c %08x", addr);
+	ret = run_commandf("fdt addr -c %08lx", addr);
 	new_fdt = gd->fdt_blob;
 	gd->fdt_blob = fdt_blob;
 	ut_assertok(ret);
@@ -187,7 +187,7 @@ static int fdt_test_addr(struct unit_test_state *uts)
 
 	/* Test setting an invalid FDT */
 	fdt[0] = 123;
-	ut_asserteq(1, run_commandf("fdt addr %08x", addr));
+	ut_asserteq(1, run_commandf("fdt addr %08lx", addr));
 	ut_assert_nextline("libfdt fdt_check_header(): FDT_ERR_BADMAGIC");
 	ut_assertok(ut_check_console_end(uts));
 
@@ -216,19 +216,19 @@ static int fdt_test_addr_resize(struct unit_test_state *uts)
 
 	/* Test setting and resizing the working FDT to a larger size */
 	ut_assertok(console_record_reset_enable());
-	ut_assertok(run_commandf("fdt addr %08x %x", addr, newsize));
+	ut_assertok(run_commandf("fdt addr %08lx %x", addr, newsize));
 	ut_assert_nextline("Working FDT set to %lx", addr);
 	ut_assertok(ut_check_console_end(uts));
 
 	/* Try shrinking it */
-	ut_assertok(run_commandf("fdt addr %08x %x", addr, sizeof(fdt) / 4));
+	ut_assertok(run_commandf("fdt addr %08lx %zx", addr, sizeof(fdt) / 4));
 	ut_assert_nextline("Working FDT set to %lx", addr);
 	ut_assert_nextline("New length %d < existing length %d, ignoring",
 			   (int)sizeof(fdt) / 4, newsize);
 	ut_assertok(ut_check_console_end(uts));
 
 	/* ...quietly */
-	ut_assertok(run_commandf("fdt addr -q %08x %x", addr, sizeof(fdt) / 4));
+	ut_assertok(run_commandf("fdt addr -q %08lx %zx", addr, sizeof(fdt) / 4));
 	ut_assert_nextline("Working FDT set to %lx", addr);
 	ut_assertok(ut_check_console_end(uts));
 
@@ -258,13 +258,13 @@ static int fdt_test_move(struct unit_test_state *uts)
 
 	/* Test moving the working FDT to a new location */
 	ut_assertok(console_record_reset_enable());
-	ut_assertok(run_commandf("fdt move %08x %08x %x", addr, newaddr, ts));
+	ut_assertok(run_commandf("fdt move %08lx %08lx %x", addr, newaddr, ts));
 	ut_assert_nextline("Working FDT set to %lx", newaddr);
 	ut_assertok(ut_check_console_end(uts));
 
 	/* Compare the source and destination DTs */
 	ut_assertok(console_record_reset_enable());
-	ut_assertok(run_commandf("cmp.b %08x %08x %x", addr, newaddr, ts));
+	ut_assertok(run_commandf("cmp.b %08lx %08lx %x", addr, newaddr, ts));
 	ut_assert_nextline("Total of %d byte(s) were the same", ts);
 	ut_assertok(ut_check_console_end(uts));
 
-- 
2.17.1

Re: [PATCH v2 3/4] test: fdt: fix run_commandf() warnings

[Simon Glass]

On Mon, 20 Mar 2023 at 21:23, Evgeny Bachinin <EABachinin@sberdevices.ru> wrote:
>
> Fix warnings both for 32bit and 64bit architecture after adding
> printf-like attribute format for run_commandf():
> warning: format ‘%x’ expects argument of type ‘unsigned int’, but
>   argument 2 has type ‘ulong {aka long unsigned int}’ [-Wformat=]
>   ret = run_commandf("fdt addr -c %08x", addr);
>                      ^
> Signed-off-by: Evgeny Bachinin <EABachinin@sberdevices.ru>
> Cc: Marek Vasut <marek.vasut+renesas@mailbox.org>
> ---
> Changes for v2:
> - fdt_test_move(): due to addition of __printf(), fix compilation
> after rebase atop 79bcd809f49 ("test: cmd: fdt: Test fdt move")
> - not apply Reviewed-by due to changes above
>
>  test/cmd/fdt.c | 16 ++++++++--------
>  1 file changed, 8 insertions(+), 8 deletions(-)
>

Reviewed-by: Simon Glass <sjg@chromium.org>

Re: [PATCH v2 3/4] test: fdt: fix run_commandf() warnings

[Tom Rini]

[-- Attachment #1: Type: text/plain, Size: 623 bytes --]

On Mon, Mar 20, 2023 at 11:23:13AM +0300, Evgeny Bachinin wrote:

> Fix warnings both for 32bit and 64bit architecture after adding
> printf-like attribute format for run_commandf():
> warning: format ‘%x’ expects argument of type ‘unsigned int’, but
>   argument 2 has type ‘ulong {aka long unsigned int}’ [-Wformat=]
>   ret = run_commandf("fdt addr -c %08x", addr);
>                      ^
> Signed-off-by: Evgeny Bachinin <EABachinin@sberdevices.ru>
> Cc: Marek Vasut <marek.vasut+renesas@mailbox.org>
> Reviewed-by: Simon Glass <sjg@chromium.org>

Applied to u-boot/next, thanks!

-- 
Tom

[-- Attachment #2: signature.asc --]
[-- Type: application/pgp-signature, Size: 659 bytes --]

[PATCH v2 4/4] test: exit: fix run_commandf() warnings

[Evgeny Bachinin]

Fix warnings after adding printf-like attribute format for
run_commandf():
warning: too many arguments for format [-Wformat-extra-args]

Signed-off-by: Evgeny Bachinin <EABachinin@sberdevices.ru>
Reviewed-by: Simon Glass <sjg@chromium.org>
---
 test/cmd/exit.c | 18 +++++++++---------
 1 file changed, 9 insertions(+), 9 deletions(-)

diff --git a/test/cmd/exit.c b/test/cmd/exit.c
index ca34abef89..7e160f7e4b 100644
--- a/test/cmd/exit.c
+++ b/test/cmd/exit.c
@@ -60,20 +60,20 @@ static int cmd_exit_test(struct unit_test_state *uts)
 
 	/* Validate that 'exit' behaves the same way as 'exit 0' */
 	ut_assertok(console_record_reset_enable());
-	ut_assertok(run_commandf("setenv foo 'echo bar ; exit ; echo baz' ; run foo ; echo $?", i));
+	ut_assertok(run_commandf("setenv foo 'echo bar ; exit ; echo baz' ; run foo ; echo $?"));
 	ut_assert_nextline("bar");
 	ut_assert_nextline("0");
 	ut_assertok(ut_check_console_end(uts));
 
 	ut_assertok(console_record_reset_enable());
-	ut_assertok(run_commandf("setenv foo 'echo bar ; exit ; echo baz' ; run foo && echo quux ; echo $?", i));
+	ut_assertok(run_commandf("setenv foo 'echo bar ; exit ; echo baz' ; run foo && echo quux ; echo $?"));
 	ut_assert_nextline("bar");
 	ut_assert_nextline("quux");
 	ut_assert_nextline("0");
 	ut_assertok(ut_check_console_end(uts));
 
 	ut_assertok(console_record_reset_enable());
-	ut_assertok(run_commandf("setenv foo 'echo bar ; exit ; echo baz' ; run foo || echo quux ; echo $?", i));
+	ut_assertok(run_commandf("setenv foo 'echo bar ; exit ; echo baz' ; run foo || echo quux ; echo $?"));
 	ut_assert_nextline("bar");
 	/* Either 'exit' returns 0, or 'echo quux' returns 0 */
 	ut_assert_nextline("0");
@@ -81,39 +81,39 @@ static int cmd_exit_test(struct unit_test_state *uts)
 
 	/* Validate that return value still propagates from 'run' command */
 	ut_assertok(console_record_reset_enable());
-	ut_assertok(run_commandf("setenv foo 'echo bar ; true' ; run foo ; echo $?", i));
+	ut_assertok(run_commandf("setenv foo 'echo bar ; true' ; run foo ; echo $?"));
 	ut_assert_nextline("bar");
 	ut_assert_nextline("0");
 	ut_assertok(ut_check_console_end(uts));
 
 	ut_assertok(console_record_reset_enable());
-	ut_assertok(run_commandf("setenv foo 'echo bar ; true' ; run foo && echo quux ; echo $?", i));
+	ut_assertok(run_commandf("setenv foo 'echo bar ; true' ; run foo && echo quux ; echo $?"));
 	ut_assert_nextline("bar");
 	ut_assert_nextline("quux");
 	ut_assert_nextline("0");
 	ut_assertok(ut_check_console_end(uts));
 
 	ut_assertok(console_record_reset_enable());
-	ut_assertok(run_commandf("setenv foo 'echo bar ; true' ; run foo || echo quux ; echo $?", i));
+	ut_assertok(run_commandf("setenv foo 'echo bar ; true' ; run foo || echo quux ; echo $?"));
 	ut_assert_nextline("bar");
 	/* The 'true' returns 0 */
 	ut_assert_nextline("0");
 	ut_assertok(ut_check_console_end(uts));
 
 	ut_assertok(console_record_reset_enable());
-	ut_assertok(run_commandf("setenv foo 'echo bar ; false' ; run foo ; echo $?", i));
+	ut_assertok(run_commandf("setenv foo 'echo bar ; false' ; run foo ; echo $?"));
 	ut_assert_nextline("bar");
 	ut_assert_nextline("1");
 	ut_assertok(ut_check_console_end(uts));
 
 	ut_assertok(console_record_reset_enable());
-	ut_assertok(run_commandf("setenv foo 'echo bar ; false' ; run foo && echo quux ; echo $?", i));
+	ut_assertok(run_commandf("setenv foo 'echo bar ; false' ; run foo && echo quux ; echo $?"));
 	ut_assert_nextline("bar");
 	ut_assert_nextline("1");
 	ut_assertok(ut_check_console_end(uts));
 
 	ut_assertok(console_record_reset_enable());
-	ut_assertok(run_commandf("setenv foo 'echo bar ; false' ; run foo || echo quux ; echo $?", i));
+	ut_assertok(run_commandf("setenv foo 'echo bar ; false' ; run foo || echo quux ; echo $?"));
 	ut_assert_nextline("bar");
 	ut_assert_nextline("quux");
 	/* The 'echo quux' returns 0 */
-- 
2.17.1

Re: [PATCH v2 4/4] test: exit: fix run_commandf() warnings

[Simon Glass]

On Mon, 20 Mar 2023 at 21:23, Evgeny Bachinin <EABachinin@sberdevices.ru> wrote:
>
> Fix warnings after adding printf-like attribute format for
> run_commandf():
> warning: too many arguments for format [-Wformat-extra-args]
>
> Signed-off-by: Evgeny Bachinin <EABachinin@sberdevices.ru>
> Reviewed-by: Simon Glass <sjg@chromium.org>
> ---
>  test/cmd/exit.c | 18 +++++++++---------
>  1 file changed, 9 insertions(+), 9 deletions(-)

Reviewed-by: Simon Glass <sjg@chromium.org>

Re: [PATCH v2 4/4] test: exit: fix run_commandf() warnings

[Tom Rini]

[-- Attachment #1: Type: text/plain, Size: 420 bytes --]

On Mon, Mar 20, 2023 at 11:23:14AM +0300, Evgeny Bachinin wrote:

> Fix warnings after adding printf-like attribute format for
> run_commandf():
> warning: too many arguments for format [-Wformat-extra-args]
> 
> Signed-off-by: Evgeny Bachinin <EABachinin@sberdevices.ru>
> Reviewed-by: Simon Glass <sjg@chromium.org>
> Reviewed-by: Simon Glass <sjg@chromium.org>

Applied to u-boot/next, thanks!

-- 
Tom

[-- Attachment #2: signature.asc --]
[-- Type: application/pgp-signature, Size: 659 bytes --]