Re: [PATCH] MdeModulePkg/UefiBootManagerLib: Limit the boot description to 72 chars

["Gao, Zhichao" <zhichao.gao@intel.com>]

Hi Ray,

Agree with you. I used to think of change the display logic of the BM app. But it is not tiny. It need to update the scroll bar definition and one more trace for the item string lines. I would prefer to defer it.
The limitation of the boot description length is required. I don't know why some USB disk would have a very long serial number string. On my opinion, 72 characters is enough for the boot option. If there is no limitation, the boot description can be long enough that one screen can not even show one item.
80 * 25 is the minimum mode for most terminal console device. That is why I choose it and add the limitation.

Thanks,
Zhichao

> -----Original Message-----
> From: Ni, Ray <ray.ni@intel.com>
> Sent: Tuesday, September 7, 2021 3:54 PM
> To: Gao, Zhichao <zhichao.gao@intel.com>; devel@edk2.groups.io
> Cc: Wang, Jian J <jian.j.wang@intel.com>; Liming Gao
> <gaoliming@byosoft.com.cn>
> Subject: RE: [PATCH] MdeModulePkg/UefiBootManagerLib: Limit the boot
> description to 72 chars
> 
> Zhichao,
> The mode 80x25 is platform specific. I don't prefer to update common logic to
> have such limitation.
> Can you update the UI logic of displaying the boot option list?
> 
> > -----Original Message-----
> > From: Gao, Zhichao <zhichao.gao@intel.com>
> > Sent: Tuesday, September 7, 2021 2:13 PM
> > To: devel@edk2.groups.io
> > Cc: Wang, Jian J <jian.j.wang@intel.com>; Liming Gao
> > <gaoliming@byosoft.com.cn>; Ni, Ray <ray.ni@intel.com>
> > Subject: [PATCH] MdeModulePkg/UefiBootManagerLib: Limit the boot
> > description to 72 chars
> >
> > REF: https://bugzilla.tianocore.org/show_bug.cgi?id=3590
> >
> > The minimum mode is 80*25 which means echo line can have 80 charactors
> > and max 25 line on the screen. And the BootManagerMeu see each boot
> > option as one line. The BootManagerMenuApp would have 2 charactors for
> > draw box and 6 charactors for space. So it is better to limit the boot
> > description within 72 charactors.
> >
> > Cc: Jian J Wang <jian.j.wang@intel.com>
> > Cc: Liming Gao <gaoliming@byosoft.com.cn>
> > Cc: Ray Ni <ray.ni@intel.com>
> > Signed-off-by: Zhichao Gao <zhichao.gao@intel.com>
> > ---
> >  .../Library/UefiBootManagerLib/BmBootDescription.c | 24
> > ++++++++++++++++++----
> >  1 file changed, 20 insertions(+), 4 deletions(-)
> >
> > diff --git
> > a/MdeModulePkg/Library/UefiBootManagerLib/BmBootDescription.c
> > b/MdeModulePkg/Library/UefiBootManagerLib/BmBootDescription.c
> > index aa891feb17..7260b2a203 100644
> > --- a/MdeModulePkg/Library/UefiBootManagerLib/BmBootDescription.c
> > +++ b/MdeModulePkg/Library/UefiBootManagerLib/BmBootDescription.c
> > @@ -1,7 +1,7 @@
> >  /** @file
> >
> >    Library functions which relate with boot option description.
> >
> >
> >
> > -Copyright (c) 2011 - 2018, Intel Corporation. All rights
> > reserved.<BR>
> >
> > +Copyright (c) 2011 - 2021, Intel Corporation. All rights
> > +reserved.<BR>
> >
> >  (C) Copyright 2015 Hewlett Packard Enterprise Development LP<BR>
> >
> >  SPDX-License-Identifier: BSD-2-Clause-Patent
> >
> >
> >
> > @@ -14,6 +14,8 @@ SPDX-License-Identifier: BSD-2-Clause-Patent
> >  #define PRODUCT_IDENTIFICATION_OFFSET    11
> >
> >  #define PRODUCT_IDENTIFICATION_LENGTH    16
> >
> >
> >
> > +#define MAX_BOOT_DESCRIPTION_LEGNTH      72
> >
> > +
> >
> >  CONST UINT16 mBmUsbLangId    = 0x0409; // English
> >
> >  CHAR16       mBmUefiPrefix[] = L"UEFI ";
> >
> >
> >
> > @@ -773,6 +775,7 @@ BmGetBootDescription (
> >    CHAR16                         *DefaultDescription;
> >
> >    CHAR16                         *Temp;
> >
> >    UINTN                          Index;
> >
> > +  UINTN                          DescriptionLen;
> >
> >
> >
> >    //
> >
> >    // Firstly get the default boot description
> >
> > @@ -785,10 +788,23 @@ BmGetBootDescription (
> >        // Avoid description confusion between UEFI & Legacy boot
> > option by adding "UEFI " prefix
> >
> >        // ONLY for core provided boot description handler.
> >
> >        //
> >
> > -      Temp = AllocatePool (StrSize (DefaultDescription) + sizeof
> (mBmUefiPrefix));
> >
> > +      if (StrLen (DefaultDescription) + StrLen (mBmUefiPrefix) >
> > + MAX_BOOT_DESCRIPTION_LEGNTH) {
> >
> > +        //
> >
> > +        // Limit the MAX length of boot description to 72 charactors.
> >
> > +        // Replace the last 3 charactors to L"...".
> >
> > +        //
> >
> > +        DescriptionLen = MAX_BOOT_DESCRIPTION_LEGNTH + 1;
> >
> > +        DefaultDescription[MAX_BOOT_DESCRIPTION_LEGNTH - 5 - 3] =
> > + L'.';
> >
> > +        DefaultDescription[MAX_BOOT_DESCRIPTION_LEGNTH - 5 - 2] =
> > + L'.';
> >
> > +        DefaultDescription[MAX_BOOT_DESCRIPTION_LEGNTH - 5 - 1] =
> > + L'.';
> >
> > +        DefaultDescription[MAX_BOOT_DESCRIPTION_LEGNTH - 5] = L'\0';
> >
> > +      } else {
> >
> > +        DescriptionLen = (StrSize (DefaultDescription) + sizeof
> > + (mBmUefiPrefix)) / sizeof (CHAR16);
> >
> > +      }
> >
> > +      Temp = AllocatePool (DescriptionLen * sizeof (CHAR16));
> >
> >        ASSERT (Temp != NULL);
> >
> > -      StrCpyS (Temp, (StrSize (DefaultDescription) + sizeof (mBmUefiPrefix))
> / sizeof (CHAR16), mBmUefiPrefix);
> >
> > -      StrCatS (Temp, (StrSize (DefaultDescription) + sizeof (mBmUefiPrefix))
> / sizeof (CHAR16), DefaultDescription);
> >
> > +      StrCpyS (Temp, DescriptionLen, mBmUefiPrefix);
> >
> > +      StrCatS (Temp, DescriptionLen, DefaultDescription);
> >
> >        FreePool (DefaultDescription);
> >
> >        DefaultDescription = Temp;
> >
> >        break;
> >
> > --
> > 2.16.2.windows.1