diff mbox series

[Concept,3/4] pager: Add length parameter to pager_post()

Message ID 20260208214411.3445278-4-sjg@u-boot.org
State New
Headers
Series Fix truetype bbox init and improve pager robustness |

Commit Message

Simon Glass Feb. 8, 2026, 9:44 p.m. UTC 
  From: Simon Glass <simon.glass@canonical.com>

Since commit 4e2b66acdd4b ("console: Add the putsn() API for
length-based string output") all puts() output goes through putsn().
When the pager is active, console_putsn_pager() cannot pass
length-delimited strings to pager_post() because it only accepts
nul-terminated strings. The workaround is character-by-character
output, which causes a severe performance regression visible in any
command that produces significant output (e.g. "dm drivers",
"dm compat") when the pager is enabled.

Rename pager_post() to pager_postn() with a new int len parameter,
so it can accept length-delimited strings directly. Add a
pager_post() inline helper for nul-terminated callers.

Update console_putsn_pager() to call through the pager path
directly, removing the s[len] == '\0' hack and the
character-by-character fallback. Rename the internal console_puts()
to console_putsn() to reflect its new length parameter.

Add a test exercising a non-nul-terminated substring.

Fixes: 4e2b66acdd4b ("console: Add the putsn() API for length-based string output")
Co-developed-by: Claude Opus 4.6 <noreply@anthropic.com>
Signed-off-by: Simon Glass <simon.glass@canonical.com>
---

 common/console.c    | 20 ++++++-----------
 common/pager.c      | 13 +++++++----
 include/pager.h     | 55 ++++++++++++++++++++++++++++++++++++++++-----
 test/common/pager.c | 19 ++++++++++++++++
 4 files changed, 85 insertions(+), 22 deletions(-)
diff mbox series

Patch

diff --git a/common/console.c b/common/console.c
index 96e8977ce06..238d464980f 100644
--- a/common/console.c
+++ b/common/console.c
@@ -361,11 +361,11 @@  int err_printf(bool serial_only, const char *fmt, ...)
 	return ret;
 }
 
-static void console_puts(int file, bool use_pager, const char *s)
+static void console_putsn(int file, bool use_pager, const char *s, int len)
 {
 	int key = 0;
 
-	for (s = pager_post(gd_pager(), use_pager, s); s;
+	for (s = pager_postn(gd_pager(), use_pager, s, len); s;
 	     s = pager_next(gd_pager(), use_pager, key)) {
 		struct stdio_dev *dev;
 		int i;
@@ -384,13 +384,8 @@  static void console_puts(int file, bool use_pager, const char *s)
 
 static void console_putsn_pager(int file, const char *s, int len)
 {
-	if (IS_ENABLED(CONFIG_CONSOLE_PAGER) && gd_pager()) {
-		/*
-		 * Pager only works with nul-terminated strings, so output
-		 * character by character for length-based strings
-		 */
-		while (len--)
-			console_putc_pager(file, *s++);
+	if (IS_ENABLED(CONFIG_CONSOLE_PAGER) && pager_active(gd_pager())) {
+		console_putsn(file, true, s, len);
 	} else {
 		struct stdio_dev *dev;
 		int i;
@@ -409,11 +404,10 @@  static void console_putsn_pager(int file, const char *s, int len)
 
 static void console_puts_pager(int file, const char *s)
 {
-	if (IS_ENABLED(CONFIG_CONSOLE_PAGER) && gd_pager()) {
-		console_puts(file, true, s);
-	} else {
+	if (IS_ENABLED(CONFIG_CONSOLE_PAGER) && gd_pager())
+		console_putsn(file, true, s, strlen(s));
+	else
 		console_putsn_pager(file, s, strlen(s));
-	}
 }
 
 #ifdef CONFIG_CONSOLE_FLUSH_SUPPORT
diff --git a/common/pager.c b/common/pager.c
index 60b1adc8571..bb8ea0776bf 100644
--- a/common/pager.c
+++ b/common/pager.c
@@ -15,10 +15,11 @@ 
 
 DECLARE_GLOBAL_DATA_PTR;
 
-const char *pager_post(struct pager *pag, bool use_pager, const char *s)
+const char *pager_postn(struct pager *pag, bool use_pager, const char *s,
+			int len)
 {
 	struct membuf old;
-	int ret, len;
+	int ret;
 
 	if (!pag || !use_pager || pag->test_bypass ||
 	    pag->state == PAGERST_BYPASS)
@@ -27,7 +28,6 @@  const char *pager_post(struct pager *pag, bool use_pager, const char *s)
 	if (pag->state == PAGERST_QUIT_SUPPRESS)
 		return NULL;
 
-	len = strlen(s);
 	if (!len)
 		return NULL;
 
@@ -42,8 +42,13 @@  const char *pager_post(struct pager *pag, bool use_pager, const char *s)
 		 * can eject the overflow text.
 		 *
 		 * The buffer is presumably empty, since callers are not allowed
-		 * to call pager_post() unless all the output from the previous
+		 * to call pager_postn() unless all the output from the previous
 		 * call was provided via pager_next().
+		 *
+		 * Note: the overflow path returns @s directly via
+		 * pager_next(), so @s must be nul-terminated. In practice
+		 * this only triggers when len > buf_size, and typical
+		 * console strings are well within the 4K default buffer.
 		 */
 		pag->overflow = s;
 		pag->mb = old;
diff --git a/include/pager.h b/include/pager.h
index 3f13f82885d..7f475787cd7 100644
--- a/include/pager.h
+++ b/include/pager.h
@@ -12,6 +12,7 @@ 
 #include <abuf.h>
 #include <membuf.h>
 #include <linux/sizes.h>
+#include <linux/string.h>
 
 #define PAGER_BUF_SIZE	SZ_4K
 
@@ -52,7 +53,7 @@  enum pager_state {
  * permit passing a string length, only a string, which means that strings must
  * be nul-terminated. The termination is handled automatically by the pager.
  *
- * If the text passed to pager_post() is too large for @buf then all the next
+ * If the text passed to pager_postn() is too large for @buf then all the next
  * will be written at once, without any paging, in the next call to
  * pager_next().
  *
@@ -85,12 +86,12 @@  struct pager {
 #if CONFIG_IS_ENABLED(CONSOLE_PAGER)
 
 /**
- * pager_post() - Add text to the input buffer for later handling
+ * pager_postn() - Add text to the input buffer for later handling
  *
  * If @use_pager the text is added to the pager buffer and fed out a screenful
- * at a time. This function calls pager_post() after storing the text.
+ * at a time. This function calls pager_postn() after storing the text.
  *
- * After calling pager_post(), if it returns anything other than NULL, you must
+ * After calling pager_postn(), if it returns anything other than NULL, you must
  * repeatedly call pager_next() until it returns NULL, otherwise text may be
  * lost
  *
@@ -99,10 +100,28 @@  struct pager {
  * @pag: Pager to use, may be NULL
  * @use_pager: Whether or not to use the pager functionality
  * @s: Text to add
+ * @len: Length of @s in bytes
  * Return: text which should be sent to output, or NULL if there is no more.
  * If !@use_pager this just returns @s and does not affect the pager state
  */
-const char *pager_post(struct pager *pag, bool use_pager, const char *s);
+const char *pager_postn(struct pager *pag, bool use_pager, const char *s,
+			int len);
+
+/**
+ * pager_post() - Add a nul-terminated string to the pager input buffer
+ *
+ * Convenience wrapper around pager_postn() for nul-terminated strings.
+ *
+ * @pag: Pager to use, may be NULL
+ * @use_pager: Whether or not to use the pager functionality
+ * @s: Nul-terminated text to add
+ * Return: text which should be sent to output, or NULL if there is no more
+ */
+static inline const char *pager_post(struct pager *pag, bool use_pager,
+				     const char *s)
+{
+	return pager_postn(pag, use_pager, s, strlen(s));
+}
 
 /**
  * pager_next() - Returns the next screenful of text to show
@@ -127,6 +146,21 @@  const char *pager_post(struct pager *pag, bool use_pager, const char *s);
  */
 const char *pager_next(struct pager *pag, bool use_pager, int ch);
 
+/**
+ * pager_active() - check if pager needs to process output
+ *
+ * Returns true only when the pager is genuinely active and needs to
+ * process output (not bypassed or in test bypass mode).
+ *
+ * @pag: Pager to check, may be NULL
+ * Return: true if the pager is active
+ */
+static inline bool pager_active(struct pager *pag)
+{
+	return pag && !pag->test_bypass &&
+	       pag->state != PAGERST_BYPASS;
+}
+
 /**
  * pager_set_bypass() - put the pager into bypass mode
  *
@@ -181,6 +215,12 @@  void pager_clear_quit(struct pager *pag);
 void pager_uninit(struct pager *pag);
 
 #else
+static inline const char *pager_postn(struct pager *pag, bool use_pager,
+				      const char *s, int len)
+{
+	return s;
+}
+
 static inline const char *pager_post(struct pager *pag, bool use_pager,
 				     const char *s)
 {
@@ -192,6 +232,11 @@  static inline const char *pager_next(struct pager *pag, bool use_pager, int ch)
 	return NULL;
 }
 
+static inline bool pager_active(struct pager *pag)
+{
+	return false;
+}
+
 static inline bool pager_set_bypass(struct pager *pag, bool bypass)
 {
 	return true;
diff --git a/test/common/pager.c b/test/common/pager.c
index ccd5230f3a4..534096a1c4f 100644
--- a/test/common/pager.c
+++ b/test/common/pager.c
@@ -665,3 +665,22 @@  static int pager_test_quit_keypress(struct unit_test_state *uts)
 	return 0;
 }
 COMMON_TEST(pager_test_quit_keypress, 0);
+
+/* Test pager with non-nul-terminated string using explicit length */
+static int pager_test_non_nul_terminated(struct unit_test_state *uts)
+{
+	struct pager *pag;
+	/* "Hello" followed by other data - not nul-terminated at offset 5 */
+	static const char data[] = "HelloWorld";
+
+	ut_assertok(pager_init(&pag, 20, 1024));
+
+	/* Post only the first 5 bytes */
+	ut_asserteq_str("Hello", pager_postn(pag, true, data, 5));
+	ut_assertnull(pager_next(pag, true, 0));
+
+	pager_uninit(pag);
+
+	return 0;
+}
+COMMON_TEST(pager_test_non_nul_terminated, 0);