[PATCH v5 10/14] efi_loader: Disable ANSI output for tests

[Simon Glass]

Hi Heinrich,

On Thu, 12 Sept 2024 at 08:58, Heinrich Schuchardt <xypron.glpk at gmx.de> wrote:
>
> On 02.09.24 03:18, Simon Glass wrote:
> > We don't want ANSI characters written in tests since it is a pain to
> > check the output with ut_assert_nextline() et al.
> >
> > Provide a way to tests to request that ANSI characters not be sent.
> >
> > Add a proper function comment while we are here, to encourage others.
> >
> > Signed-off-by: Simon Glass <sjg at chromium.org>
>
>
> This patch was already NAKed in the last version.
>
> Please, adjust the test framework if you need filtering.

In what way should it be adjusted? I hope you are not suggesting that
sandbox should try to filter out ANSI characters in its assertions?

We need our code to be designed around testing. The current code
blindly sends out ANSI codes without no knowledge of whether there is
anything at the other end which understands them.

Anyway, please make a concrete suggestion. At this point, at v5, I
would just like this series applied ASAP with minimal extra effort.

Regards,
Simon






>
> Best regards
>
> Heinrich
>
> > ---
> >
> > (no changes since v1)
> >
> >   include/efi_loader.h         | 21 ++++++++++++++++++++-
> >   lib/efi_loader/efi_console.c | 26 +++++++++++++++++---------
> >   2 files changed, 37 insertions(+), 10 deletions(-)
> >
> > diff --git a/include/efi_loader.h b/include/efi_loader.h
> > index f84852e384f..82b90ee0f1d 100644
> > --- a/include/efi_loader.h
> > +++ b/include/efi_loader.h
> > @@ -531,8 +531,27 @@ efi_status_t efi_bootmgr_delete_boot_option(u16 boot_index);
> >   efi_status_t efi_bootmgr_run(void *fdt);
> >   /* search the boot option index in BootOrder */
> >   bool efi_search_bootorder(u16 *bootorder, efi_uintn_t num, u32 target, u32 *index);
> > -/* Set up console modes */
> > +
> > +/**
> > + * efi_setup_console_size() - update the mode table.
> > + *
> > + * By default the only mode available is 80x25. If the console has at least 50
> > + * lines, enable mode 80x50. If we can query the console size and it is neither
> > + * 80x25 nor 80x50, set it as an additional mode.
> > + */
> >   void efi_setup_console_size(void);
> > +
> > +/**
> > + * efi_console_set_ansi() - Set whether ANSI characters should be emitted
> > + *
> > + * These characters mess up tests which use ut_assert_nextline(). Call this
> > + * function to tell efi_loader not to emit these characters when starting up the
> > + * terminal
> > + *
> > + * @allow_ansi: Allow emitting ANSI characters
> > + */
> > +void efi_console_set_ansi(bool allow_ansi);
> > +
> >   /* Set up load options from environment variable */
> >   efi_status_t efi_env_set_load_options(efi_handle_t handle, const char *env_var,
> >                                     u16 **load_options);
> > diff --git a/lib/efi_loader/efi_console.c b/lib/efi_loader/efi_console.c
> > index cea50c748aa..569fc9199bc 100644
> > --- a/lib/efi_loader/efi_console.c
> > +++ b/lib/efi_loader/efi_console.c
> > @@ -30,6 +30,17 @@ struct cout_mode {
> >
> >   __maybe_unused static struct efi_object uart_obj;
> >
> > +/*
> > + * suppress emission of ANSI codes for use by unit tests. Leave it as 0 for the
> > + * default behaviour
> > + */
> > +static bool no_ansi;
> > +
> > +void efi_console_set_ansi(bool allow_ansi)
> > +{
> > +     no_ansi = !allow_ansi;
> > +}
> > +
> >   static struct cout_mode efi_cout_modes[] = {
> >       /* EFI Mode 0 is 80x25 and always present */
> >       {
> > @@ -348,13 +359,6 @@ static int __maybe_unused query_vidconsole(int *rows, int *cols)
> >       return 0;
> >   }
> >
> > -/**
> > - * efi_setup_console_size() - update the mode table.
> > - *
> > - * By default the only mode available is 80x25. If the console has at least 50
> > - * lines, enable mode 80x50. If we can query the console size and it is neither
> > - * 80x25 nor 80x50, set it as an additional mode.
> > - */
> >   void efi_setup_console_size(void)
> >   {
> >       int rows = 25, cols = 80;
> > @@ -362,8 +366,12 @@ void efi_setup_console_size(void)
> >
> >       if (IS_ENABLED(CONFIG_VIDEO))
> >               ret = query_vidconsole(&rows, &cols);
> > -     if (ret)
> > -             ret = query_console_serial(&rows, &cols);
> > +     if (ret) {
> > +             if (no_ansi)
> > +                     ret = 0;
> > +             else
> > +                     ret = query_console_serial(&rows, &cols);
> > +     }
> >       if (ret)
> >               return;
> >
>