[PATCH v3 1/3] efi_loader: nocolor command line attr for initrddump.efi

[Ilias Apalodimas]

Hi Heinrich,

On Tue, 22 Mar 2022 at 22:58, Heinrich Schuchardt
<heinrich.schuchardt at canonical.com> wrote:
>
> initrddump.efi uses colored output and clear the screen. This is not
> helpful for integration into Python tests. Allow specifying 'nocolor' in
> the load option data to suppress color output and clearing the screen.
>
> Signed-off-by: Heinrich Schuchardt <heinrich.schuchardt at canonical.com>
> ---
> v3:
>         new patch
> ---
>  lib/efi_loader/initrddump.c | 77 ++++++++++++++++++++++++++++++++-----
>  1 file changed, 68 insertions(+), 9 deletions(-)
>
> diff --git a/lib/efi_loader/initrddump.c b/lib/efi_loader/initrddump.c
> index 7de43bcfff..25dbd635e5 100644
> --- a/lib/efi_loader/initrddump.c
> +++ b/lib/efi_loader/initrddump.c
> @@ -4,6 +4,9 @@
>   *
>   * initrddump.efi saves the initial RAM disk provided via the
>   * EFI_LOAD_FILE2_PROTOCOL.
> + *
> + * Specifying 'nocolor' as load option data suppresses colored output and
> + * clearing of the screen.
>   */
>
>  #include <common.h>
> @@ -25,6 +28,7 @@ static const efi_guid_t guid_simple_file_system_protocol =
>                                         EFI_SIMPLE_FILE_SYSTEM_PROTOCOL_GUID;
>  static const efi_guid_t load_file2_guid = EFI_LOAD_FILE2_PROTOCOL_GUID;
>  static efi_handle_t handle;
> +static bool nocolor;
>
>  /*
>   * Device path defined by Linux to identify the handle providing the
> @@ -46,6 +50,17 @@ static const struct efi_initrd_dp initrd_dp = {
>         }
>  };
>
> +/**
> + * color() - set foreground color
> + *
> + * @color:     foreground color
> + */
> +static void color(u8 color)
> +{
> +       if (!nocolor)
> +               cout->set_attribute(cout, color | EFI_BACKGROUND_BLACK);
> +}
> +
>  /**
>   * print() - print string
>   *
> @@ -56,6 +71,17 @@ static void print(u16 *string)
>         cout->output_string(cout, string);
>  }
>
> +/**
> + * cls() - clear screen
> + */
> +static void cls(void)
> +{
> +       if (nocolor)
> +               print(u"\r\n");
> +       else
> +               cout->clear_screen(cout);
> +}
> +
>  /**
>   * error() - print error string
>   *
> @@ -63,9 +89,9 @@ static void print(u16 *string)
>   */
>  static void error(u16 *string)
>  {
> -       cout->set_attribute(cout, EFI_LIGHTRED | EFI_BACKGROUND_BLACK);
> +       color(EFI_LIGHTRED);
>         print(string);
> -       cout->set_attribute(cout, EFI_LIGHTBLUE | EFI_BACKGROUND_BLACK);
> +       color(EFI_LIGHTBLUE);
>  }
>
>  /*
> @@ -215,10 +241,13 @@ static u16 *skip_whitespace(u16 *pos)
>   *
>   * @string:    string to search for keyword
>   * @keyword:   keyword to be searched
> - * Return:     true fi @string starts with the keyword
> + * Return:     true if @string starts with the keyword
>   */
>  static bool starts_with(u16 *string, u16 *keyword)
>  {
> +       if (!string || !keyword)
> +               return false;
> +
>         for (; *keyword; ++string, ++keyword) {
>                 if (*string != *keyword)
>                         return false;
> @@ -400,6 +429,30 @@ out:
>         return ret;
>  }
>
> +/**
> + * get_load_options() - get load options
> + *
> + * Return:     load options or NULL
> + */
> +u16 *get_load_options(void)
> +{
> +       efi_status_t ret;
> +       struct efi_loaded_image *loaded_image;
> +
> +       ret = bs->open_protocol(handle, &loaded_image_guid,
> +                               (void **)&loaded_image, NULL, NULL,
> +                               EFI_OPEN_PROTOCOL_GET_PROTOCOL);
> +       if (ret != EFI_SUCCESS) {
> +               error(u"Loaded image protocol not found\r\n");
> +               return NULL;
> +       }
> +
> +       if (!loaded_image->load_options_size || !loaded_image->load_options)
> +               return NULL;
> +
> +       return loaded_image->load_options;
> +}
> +
>  /**
>   * efi_main() - entry point of the EFI application.
>   *
> @@ -410,18 +463,23 @@ out:
>  efi_status_t EFIAPI efi_main(efi_handle_t image_handle,
>                              struct efi_system_table *systab)
>  {
> +       u16 *load_options;
> +
>         handle = image_handle;
>         systable = systab;
>         cerr = systable->std_err;
>         cout = systable->con_out;
>         cin = systable->con_in;
>         bs = systable->boottime;
> +       load_options = get_load_options();

Don't we need to check for NULL here?

>
> -       cout->set_attribute(cout, EFI_LIGHTBLUE | EFI_BACKGROUND_BLACK);
> -       cout->clear_screen(cout);
> -       cout->set_attribute(cout, EFI_WHITE | EFI_BACKGROUND_BLACK);
> +       if (starts_with(load_options, u"nocolor"))
> +               nocolor = true;
> +
> +       color(EFI_WHITE);
> +       cls();
>         print(u"INITRD Dump\r\n===========\r\n\r\n");
> -       cout->set_attribute(cout, EFI_LIGHTBLUE | EFI_BACKGROUND_BLACK);
> +       color(EFI_LIGHTBLUE);
>
>         for (;;) {
>                 u16 command[BUFFER_SIZE];
> @@ -443,7 +501,8 @@ efi_status_t EFIAPI efi_main(efi_handle_t image_handle,
>                         do_help();
>         }
>
> -       cout->set_attribute(cout, EFI_LIGHTGRAY | EFI_BACKGROUND_BLACK);
> -       cout->clear_screen(cout);
> +       color(EFI_LIGHTGRAY);
> +       cls();
> +
>         return EFI_SUCCESS;
>  }
> --
> 2.34.1
>

Cheers
/Ilias