Re: [PATCH] cli: flush stdin before enabling cli
Hi Tom,
> Conceptually, good idea, thanks for doing it.
> >
> > ---
> >
> > cmd/Kconfig | 7 +++
> > common/cli.c | 6 ++
> > 2 files changed, 13 insertions(+)
> >
> > diff --git a/cmd/Kconfig b/cmd/Kconfig
> > index 322ebe600c5..e08fbf27358 100644
> > --- a/cmd/Kconfig
> > +++ b/cmd/Kconfig
> > @@ -47,6 +47,13 @@ config HUSH_SELECTABLE
> > default y if HUSH_OLD_PARSER && HUSH_MODERN_PARSER
> > endmenu
> >
> > +config CMDLINE_FLUSH_STDIN
> > + bool "Enable flushing input before starting cli"
> > + default n
>
> We don't need default n as that's the default.
> > + if (IS_ENABLED(CONFIG_CMDLINE_FLUSH_STDIN)) {
> > + while (tstc())
> > + (void)getchar();
> > + }
> > +
> > #if CONFIG_IS_ENABLED(HUSH_PARSER)
> > if (gd->flags & GD_FLG_HUSH_MODERN_PARSER)
> > parse_and_run_file();
>
> This is flush_stdin() from common/autoboot.c, but that's a static
> function. We should find a better spot for that perhaps and then call it
> when required.
Thanks for the review. I will send a v2 to change this.
Gregor
Re: [PATCH] cli: flush stdin before enabling cli
> > Currently there is no possibility to flush stdin after autocommands are > > executed. If in the bootcmd the stdin is changed, e.g. from nulldev to > > serial, it could happen that junk characters sit in the fifo and appear > > on the cli. > > > > Add a option to clear stdin before starting the CLI. > > > > Why does this need an option? I don't know enough (anything really) about > the cli in general but the wording seems to indicate this fixes an issue or > even a bug. Is there a case in which we wouldn't want to flush? In our case the problem was in efi_console where query_console_serial sent the escape sequence to stdout and stdin was set to nulldev and therefore never received. In the bootcmd stdin was set to serial and the response was seen on the cli. Thats how it was in our case. My first approach was to check in efi_console to not send if stdin is nulldev. But after more thoughts I implemented the current approach which flushes stdin before starting cli. After all efi_console did exactly what was configured in the environment. I can think of two cases where stdin shouldn't be flushed: * User typing commands before cli is reached. * Some scripts sending commands before cli prompt is reached. Imho there should be at least an option to let users decide on there usecase. If the option is default or not I don't have a strong opinion on. Gregor
Re: [PATCH] cli: flush stdin before enabling cli
Hi Gregor, On 4/1/26 12:46 PM, Gregor Herburger wrote: [You don't often get email from [email protected]. Learn why this is important at https://aka.ms/LearnAboutSenderIdentification ] Currently there is no possibility to flush stdin after autocommands are executed. If in the bootcmd the stdin is changed, e.g. from nulldev to serial, it could happen that junk characters sit in the fifo and appear on the cli. Add a option to clear stdin before starting the CLI. Why does this need an option? I don't know enough (anything really) about the cli in general but the wording seems to indicate this fixes an issue or even a bug. Is there a case in which we wouldn't want to flush? In our case the problem was in efi_console where query_console_serial sent the escape sequence to stdout and stdin was set to nulldev and therefore never received. In the bootcmd stdin was set to serial and the response was seen on the cli. Thats how it was in our case. My first approach was to check in efi_console to not send if stdin is nulldev. But after more thoughts I implemented the current approach which flushes stdin before starting cli. After all efi_console did exactly what was configured in the environment. I can think of two cases where stdin shouldn't be flushed: * User typing commands before cli is reached. I see we have flush_stdin() in common/autoboot.c already, with the same implementation. Guarded by CONFIG_AUTOBOOT_FLUSH_STDIN. So I guess if we have a knob for that, another knob for the cli_loop() part will at least be consistent so fine by me. I kinda fail to see why we would not want to flush, but /me shrugs. Cheers, Quentin
Re: [PATCH] cli: flush stdin before enabling cli
Hi Gregor, On 3/31/26 1:03 PM, Gregor Herburger wrote: [You don't often get email from [email protected]. Learn why this is important at https://aka.ms/LearnAboutSenderIdentification ] Currently there is no possibility to flush stdin after autocommands are executed. If in the bootcmd the stdin is changed, e.g. from nulldev to serial, it could happen that junk characters sit in the fifo and appear on the cli. Add a option to clear stdin before starting the CLI. Why does this need an option? I don't know enough (anything really) about the cli in general but the wording seems to indicate this fixes an issue or even a bug. Is there a case in which we wouldn't want to flush? Cheers, Quentin
Re: [PATCH] cli: flush stdin before enabling cli
On Tue, Mar 31, 2026 at 01:03:08PM +0200, Gregor Herburger wrote:
> Currently there is no possibility to flush stdin after autocommands are
> executed. If in the bootcmd the stdin is changed, e.g. from nulldev to
> serial, it could happen that junk characters sit in the fifo and appear
> on the cli.
>
> Add a option to clear stdin before starting the CLI.
>
> Signed-off-by: Gregor Herburger
Conceptually, good idea, thanks for doing it.
>
> ---
>
> cmd/Kconfig | 7 +++
> common/cli.c | 6 ++
> 2 files changed, 13 insertions(+)
>
> diff --git a/cmd/Kconfig b/cmd/Kconfig
> index 322ebe600c5..e08fbf27358 100644
> --- a/cmd/Kconfig
> +++ b/cmd/Kconfig
> @@ -47,6 +47,13 @@ config HUSH_SELECTABLE
> default y if HUSH_OLD_PARSER && HUSH_MODERN_PARSER
> endmenu
>
> +config CMDLINE_FLUSH_STDIN
> + bool "Enable flushing input before starting cli"
> + default n
We don't need default n as that's the default.
> + help
> + When this option is enabled the stdin buffer will be flushed before
> + starting the CLI.
> +
> config CMDLINE_EDITING
> bool "Enable command line editing"
> default y
> diff --git a/common/cli.c b/common/cli.c
> index 4694a35cd0e..42d27ed87e7 100644
> --- a/common/cli.c
> +++ b/common/cli.c
> @@ -295,6 +295,12 @@ err:
> void cli_loop(void)
> {
> bootstage_mark(BOOTSTAGE_ID_ENTER_CLI_LOOP);
> +
> + if (IS_ENABLED(CONFIG_CMDLINE_FLUSH_STDIN)) {
> + while (tstc())
> + (void)getchar();
> + }
> +
> #if CONFIG_IS_ENABLED(HUSH_PARSER)
> if (gd->flags & GD_FLG_HUSH_MODERN_PARSER)
> parse_and_run_file();
This is flush_stdin() from common/autoboot.c, but that's a static
function. We should find a better spot for that perhaps and then call it
when required.
--
Tom
signature.asc
Description: PGP signature
