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 --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); -- 2.43.0