From: Simon Glass <simon.glass@canonical.com> Currently pager_next() relies on a `buf.size - 1` limit in the membuf_getraw() call to leave room for the nul terminator it writes. This is fragile because the reservation happens at every read rather than being enforced structurally. Initialise the membuf one byte smaller than the abuf so the last byte is always available for the nul write. The getraw call can then simply request all available data, and the nul terminator always falls within the abuf. Co-developed-by: Claude Opus 4.6 <noreply@anthropic.com> --- common/console.c | 20 ++++++--- common/pager.c | 13 ++---- include/pager.h | 55 +++--------------------- test/common/pager.c | 19 -------- test/hooks/bin/ellesmere/conf.malta_qemu | 1 - 5 files changed, 22 insertions(+), 86 deletions(-) delete mode 120000 test/hooks/bin/ellesmere/conf.malta_qemu diff --git a/common/console.c b/common/console.c index 238d464980f..96e8977ce06 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_putsn(int file, bool use_pager, const char *s, int len) +static void console_puts(int file, bool use_pager, const char *s) { int key = 0; - for (s = pager_postn(gd_pager(), use_pager, s, len); s; + for (s = pager_post(gd_pager(), use_pager, s); s; s = pager_next(gd_pager(), use_pager, key)) { struct stdio_dev *dev; int i; @@ -384,8 +384,13 @@ static void console_putsn(int file, bool use_pager, const char *s, int len) static void console_putsn_pager(int file, const char *s, int len) { - if (IS_ENABLED(CONFIG_CONSOLE_PAGER) && pager_active(gd_pager())) { - console_putsn(file, true, s, 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++); } else { struct stdio_dev *dev; int i; @@ -404,10 +409,11 @@ 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_putsn(file, true, s, strlen(s)); - else + if (IS_ENABLED(CONFIG_CONSOLE_PAGER) && gd_pager()) { + console_puts(file, true, 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 bb8ea0776bf..60b1adc8571 100644 --- a/common/pager.c +++ b/common/pager.c @@ -15,11 +15,10 @@ DECLARE_GLOBAL_DATA_PTR; -const char *pager_postn(struct pager *pag, bool use_pager, const char *s, - int len) +const char *pager_post(struct pager *pag, bool use_pager, const char *s) { struct membuf old; - int ret; + int ret, len; if (!pag || !use_pager || pag->test_bypass || pag->state == PAGERST_BYPASS) @@ -28,6 +27,7 @@ const char *pager_postn(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,13 +42,8 @@ const char *pager_postn(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_postn() unless all the output from the previous + * to call pager_post() 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 7f475787cd7..3f13f82885d 100644 --- a/include/pager.h +++ b/include/pager.h @@ -12,7 +12,6 @@ #include <abuf.h> #include <membuf.h> #include <linux/sizes.h> -#include <linux/string.h> #define PAGER_BUF_SIZE SZ_4K @@ -53,7 +52,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_postn() is too large for @buf then all the next + * If the text passed to pager_post() is too large for @buf then all the next * will be written at once, without any paging, in the next call to * pager_next(). * @@ -86,12 +85,12 @@ struct pager { #if CONFIG_IS_ENABLED(CONSOLE_PAGER) /** - * pager_postn() - Add text to the input buffer for later handling + * pager_post() - 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_postn() after storing the text. + * at a time. This function calls pager_post() after storing the text. * - * After calling pager_postn(), if it returns anything other than NULL, you must + * After calling pager_post(), if it returns anything other than NULL, you must * repeatedly call pager_next() until it returns NULL, otherwise text may be * lost * @@ -100,28 +99,10 @@ 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_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)); -} +const char *pager_post(struct pager *pag, bool use_pager, const char *s); /** * pager_next() - Returns the next screenful of text to show @@ -146,21 +127,6 @@ static inline const char *pager_post(struct pager *pag, bool use_pager, */ 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 * @@ -215,12 +181,6 @@ 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) { @@ -232,11 +192,6 @@ 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 534096a1c4f..ccd5230f3a4 100644 --- a/test/common/pager.c +++ b/test/common/pager.c @@ -665,22 +665,3 @@ 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); diff --git a/test/hooks/bin/ellesmere/conf.malta_qemu b/test/hooks/bin/ellesmere/conf.malta_qemu deleted file mode 120000 index ded626f8191..00000000000 --- a/test/hooks/bin/ellesmere/conf.malta_qemu +++ /dev/null @@ -1 +0,0 @@ -../travis-ci/conf.malta_qemu \ No newline at end of file -- 2.43.0