1. Patchew
  2. EDK2
  3. View series

[edk2-devel] [PATCH] MdeModulePkg/UefiBootManagerLib: Limit the boot description to 72 chars

Gao, Zhichao posted 1 patch 2 years, 7 months ago
Patches applied successfully (tree, apply log)
git fetch https://github.com/patchew-project/edk2 tags/patchew/20210907061252.2069-1-zhichao.gao@intel.com
.../Library/UefiBootManagerLib/BmBootDescription.c | 24 ++++++++++++++++++----
1 file changed, 20 insertions(+), 4 deletions(-)
[edk2-devel] [PATCH] MdeModulePkg/UefiBootManagerLib: Limit the boot description to 72 chars
Posted by Gao, Zhichao 2 years, 7 months ago 
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



-=-=-=-=-=-=-=-=-=-=-=-
Groups.io Links: You receive all messages sent to this group.
View/Reply Online (#80287): https://edk2.groups.io/g/devel/message/80287
Mute This Topic: https://groups.io/mt/85429661/1787277
Group Owner: devel+owner@edk2.groups.io
Unsubscribe: https://edk2.groups.io/g/devel/unsub [importer@patchew.org]
-=-=-=-=-=-=-=-=-=-=-=-
Re: [edk2-devel] [PATCH] MdeModulePkg/UefiBootManagerLib: Limit the boot description to 72 chars
Posted by Ni, Ray 2 years, 7 months ago 
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



-=-=-=-=-=-=-=-=-=-=-=-
Groups.io Links: You receive all messages sent to this group.
View/Reply Online (#80292): https://edk2.groups.io/g/devel/message/80292
Mute This Topic: https://groups.io/mt/85429661/1787277
Group Owner: devel+owner@edk2.groups.io
Unsubscribe: https://edk2.groups.io/g/devel/unsub [importer@patchew.org]
-=-=-=-=-=-=-=-=-=-=-=-
Re: [edk2-devel] [PATCH] MdeModulePkg/UefiBootManagerLib: Limit the boot description to 72 chars
Posted by Gao, Zhichao 2 years, 7 months ago 
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



-=-=-=-=-=-=-=-=-=-=-=-
Groups.io Links: You receive all messages sent to this group.
View/Reply Online (#80317): https://edk2.groups.io/g/devel/message/80317
Mute This Topic: https://groups.io/mt/85429661/1787277
Group Owner: devel+owner@edk2.groups.io
Unsubscribe: https://edk2.groups.io/g/devel/unsub [importer@patchew.org]
-=-=-=-=-=-=-=-=-=-=-=-
Re: [edk2-devel] [PATCH] MdeModulePkg/UefiBootManagerLib: Limit the boot description to 72 chars
Posted by Ni, Ray 2 years, 7 months ago 
Zhichao,
I didn't mean to update UI logic to support scrolling. I meant to update BM logic to show partial string of boot description.

Thanks,
Ray

> -----Original Message-----
> From: Gao, Zhichao <zhichao.gao@intel.com>
> Sent: Wednesday, September 8, 2021 10:29 AM
> To: Ni, Ray <ray.ni@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
> 
> 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



-=-=-=-=-=-=-=-=-=-=-=-
Groups.io Links: You receive all messages sent to this group.
View/Reply Online (#80373): https://edk2.groups.io/g/devel/message/80373
Mute This Topic: https://groups.io/mt/85429661/1787277
Group Owner: devel+owner@edk2.groups.io
Unsubscribe: https://edk2.groups.io/g/devel/unsub [importer@patchew.org]
-=-=-=-=-=-=-=-=-=-=-=-
Re: [edk2-devel] [PATCH] MdeModulePkg/UefiBootManagerLib: Limit the boot description to 72 chars
Posted by Gao, Zhichao 2 years, 7 months ago 
Well. That's another solution to limit the boot option showing in one line. I can implement this.

Thanks,
Zhichao

> -----Original Message-----
> From: Ni, Ray <ray.ni@intel.com>
> Sent: Thursday, September 9, 2021 1:02 AM
> 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,
> I didn't mean to update UI logic to support scrolling. I meant to update BM
> logic to show partial string of boot description.
> 
> Thanks,
> Ray
> 
> > -----Original Message-----
> > From: Gao, Zhichao <zhichao.gao@intel.com>
> > Sent: Wednesday, September 8, 2021 10:29 AM
> > To: Ni, Ray <ray.ni@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
> >
> > 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



-=-=-=-=-=-=-=-=-=-=-=-
Groups.io Links: You receive all messages sent to this group.
View/Reply Online (#80405): https://edk2.groups.io/g/devel/message/80405
Mute This Topic: https://groups.io/mt/85429661/1787277
Group Owner: devel+owner@edk2.groups.io
Unsubscribe: https://edk2.groups.io/g/devel/unsub [importer@patchew.org]
-=-=-=-=-=-=-=-=-=-=-=-

Patchew 2.1-devel-0870f84

© 2016 - 2024 Red Hat, Inc.