U-Boot Archive on lore.kernel.org
 help / color / mirror / Atom feed
From: Evgeny Bachinin <EABachinin@sberdevices.ru>
To: Simon Glass <sjg@chromium.org>,
	Hector Palacios <hector.palacios@digi.com>,
	Marek Vasut <marex@denx.de>,
	Heinrich Schuchardt <xypron.glpk@gmx.de>,
	John Keeping <john@metanate.com>
Cc: <u-boot@lists.denx.de>, <kernel@sberdevices.ru>,
	<evgen89bachinin@gmail.com>,
	Evgeny Bachinin <EABachinin@sberdevices.ru>
Subject: [PATCH v2 1/4] cli: run_commandf(): small fixups
Date: Mon, 20 Mar 2023 11:23:11 +0300	[thread overview]
Message-ID: <20230320082314.2018-2-EABachinin@sberdevices.ru> (raw)
In-Reply-To: <20230320082314.2018-1-EABachinin@sberdevices.ru>

* 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


  reply	other threads:[~2023-03-20  8:23 UTC|newest]

Thread overview: 13+ messages / expand[flat|nested]  mbox.gz  Atom feed  top
2023-03-20  8:23 [PATCH v2 0/4] cli: run_commandf() coverage and small fixups Evgeny Bachinin
2023-03-20  8:23 ` Evgeny Bachinin [this message]
2023-03-20 18:40   ` [PATCH v2 1/4] cli: run_commandf(): " Simon Glass
2023-03-31 14:15   ` Tom Rini
2023-03-20  8:23 ` [PATCH v2 2/4] unit-test: cover run_commandf() by test-cases Evgeny Bachinin
2023-03-20 18:40   ` Simon Glass
2023-03-31 14:15   ` Tom Rini
2023-03-20  8:23 ` [PATCH v2 3/4] test: fdt: fix run_commandf() warnings Evgeny Bachinin
2023-03-20 18:40   ` Simon Glass
2023-03-31 14:15   ` Tom Rini
2023-03-20  8:23 ` [PATCH v2 4/4] test: exit: " Evgeny Bachinin
2023-03-20 18:40   ` Simon Glass
2023-03-31 14:15   ` Tom Rini

Reply instructions:

You may reply publicly to this message via plain-text email
using any one of the following methods:

* Save the following mbox file, import it into your mail client,
  and reply-to-all from there: mbox

  Avoid top-posting and favor interleaved quoting:
  https://en.wikipedia.org/wiki/Posting_style#Interleaved_style

* Reply using the --to, --cc, and --in-reply-to
  switches of git-send-email(1):

  git send-email \
    --in-reply-to=20230320082314.2018-2-EABachinin@sberdevices.ru \
    --to=eabachinin@sberdevices.ru \
    --cc=evgen89bachinin@gmail.com \
    --cc=hector.palacios@digi.com \
    --cc=john@metanate.com \
    --cc=kernel@sberdevices.ru \
    --cc=marex@denx.de \
    --cc=sjg@chromium.org \
    --cc=u-boot@lists.denx.de \
    --cc=xypron.glpk@gmx.de \
    /path/to/YOUR_REPLY

  https://kernel.org/pub/software/scm/git/docs/git-send-email.html

* If your mail client supports setting the In-Reply-To header
  via mailto: links, try the mailto: link
Be sure your reply has a Subject: header at the top and a blank line before the message body.
This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox