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

[Heinrich Schuchardt]

On 3/25/22 15:58, Ilias Apalodimas wrote:
> 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?

Thanks for reviewing.

The check is in starts_with().

Best regards

Heinrich

> 
>>
>> -       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