* [PATCH] MdeModulePkg/UefiBootManagerLib: Limit the boot description to 72 chars @ 2021-09-07 6:12 Gao, Zhichao 2021-09-07 7:53 ` Ni, Ray 0 siblings, 1 reply; 6+ messages in thread From: Gao, Zhichao @ 2021-09-07 6:12 UTC (permalink / raw) To: devel; +Cc: Jian J Wang, Liming Gao, Ray Ni 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 ^ permalink raw reply related [flat|nested] 6+ messages in thread
* Re: [PATCH] MdeModulePkg/UefiBootManagerLib: Limit the boot description to 72 chars 2021-09-07 6:12 [PATCH] MdeModulePkg/UefiBootManagerLib: Limit the boot description to 72 chars Gao, Zhichao @ 2021-09-07 7:53 ` Ni, Ray 2021-09-08 2:28 ` Gao, Zhichao 0 siblings, 1 reply; 6+ messages in thread From: Ni, Ray @ 2021-09-07 7:53 UTC (permalink / raw) To: Gao, Zhichao, devel@edk2.groups.io; +Cc: Wang, Jian J, Liming Gao 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 ^ permalink raw reply [flat|nested] 6+ messages in thread
* Re: [PATCH] MdeModulePkg/UefiBootManagerLib: Limit the boot description to 72 chars 2021-09-07 7:53 ` Ni, Ray @ 2021-09-08 2:28 ` Gao, Zhichao 2021-09-08 17:01 ` Ni, Ray 0 siblings, 1 reply; 6+ messages in thread From: Gao, Zhichao @ 2021-09-08 2:28 UTC (permalink / raw) To: Ni, Ray, devel@edk2.groups.io; +Cc: Wang, Jian J, Liming Gao 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 ^ permalink raw reply [flat|nested] 6+ messages in thread
* Re: [PATCH] MdeModulePkg/UefiBootManagerLib: Limit the boot description to 72 chars 2021-09-08 2:28 ` Gao, Zhichao @ 2021-09-08 17:01 ` Ni, Ray 2021-09-09 6:12 ` Gao, Zhichao [not found] ` <16A312A11F4E487B.29348@groups.io> 0 siblings, 2 replies; 6+ messages in thread From: Ni, Ray @ 2021-09-08 17:01 UTC (permalink / raw) To: Gao, Zhichao, devel@edk2.groups.io; +Cc: Wang, Jian J, Liming Gao 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 ^ permalink raw reply [flat|nested] 6+ messages in thread
* Re: [PATCH] MdeModulePkg/UefiBootManagerLib: Limit the boot description to 72 chars 2021-09-08 17:01 ` Ni, Ray @ 2021-09-09 6:12 ` Gao, Zhichao [not found] ` <16A312A11F4E487B.29348@groups.io> 1 sibling, 0 replies; 6+ messages in thread From: Gao, Zhichao @ 2021-09-09 6:12 UTC (permalink / raw) To: Ni, Ray, devel@edk2.groups.io; +Cc: Wang, Jian J, Liming Gao 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 ^ permalink raw reply [flat|nested] 6+ messages in thread
[parent not found: <16A312A11F4E487B.29348@groups.io>]
* Re: [edk2-devel] [PATCH] MdeModulePkg/UefiBootManagerLib: Limit the boot description to 72 chars [not found] ` <16A312A11F4E487B.29348@groups.io> @ 2021-09-09 8:13 ` Gao, Zhichao 0 siblings, 0 replies; 6+ messages in thread From: Gao, Zhichao @ 2021-09-09 8:13 UTC (permalink / raw) To: devel@edk2.groups.io, Gao, Zhichao, Ni, Ray; +Cc: Wang, Jian J, Liming Gao I already sent the V2. But it would have the affection I mention in the Bugzilla: If just limit it when show it, it would cause the different showing in Setup Boot manager page with BootManagerMenu. Thanks, Zhichao > -----Original Message----- > From: devel@edk2.groups.io <devel@edk2.groups.io> On Behalf Of Gao, > Zhichao > Sent: Thursday, September 9, 2021 2:13 PM > 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: [edk2-devel] [PATCH] MdeModulePkg/UefiBootManagerLib: Limit > the boot description to 72 chars > > 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 > > > > > > ^ permalink raw reply [flat|nested] 6+ messages in thread
end of thread, other threads:[~2021-09-09 8:14 UTC | newest]
Thread overview: 6+ messages (download: mbox.gz follow: Atom feed
-- links below jump to the message on this page --
2021-09-07 6:12 [PATCH] MdeModulePkg/UefiBootManagerLib: Limit the boot description to 72 chars Gao, Zhichao
2021-09-07 7:53 ` Ni, Ray
2021-09-08 2:28 ` Gao, Zhichao
2021-09-08 17:01 ` Ni, Ray
2021-09-09 6:12 ` Gao, Zhichao
[not found] ` <16A312A11F4E487B.29348@groups.io>
2021-09-09 8:13 ` [edk2-devel] " Gao, Zhichao
This is a public inbox, see mirroring instructions for how to clone and mirror all data and code used for this inbox