Thanks Liming From: gaoliming [mailto:gaoliming@byosoft.com.cn] Sent: Friday, February 5, 2021 11:52 AM To: devel@edk2.groups.io; Chang, Abner (HPS SW/FW Technologist) ; dandan.bi@intel.com Cc: 'Dong, Eric' ; Wang, Nickle (HPS SW) Subject: 回复: [edk2-devel] [PATCH v3] MdeModulePkg/Library: Add HiiGetStringEx to UefiHiiLib for EDK2 Redfish I am OK for any option. Reviewed-by: Liming Gao > Thanks LIming 发件人: bounce+27952+71277+4905953+8761045@groups.io > 代表 Abner Chang 发送时间: 2021年2月5日 9:51 收件人: devel@edk2.groups.io; Chang, Abner (HPS SW/FW Technologist) >; dandan.bi@intel.com 抄送: 'Dong, Eric' >; Wang, Nickle (HPS SW) > 主题: Re: [edk2-devel] [PATCH v3] MdeModulePkg/Library: Add HiiGetStringEx to UefiHiiLib for EDK2 Redfish Hi Liming, Do you agree to keep the one Dandan suggested? Regards, Abner From: devel@edk2.groups.io [mailto:devel@edk2.groups.io] On Behalf Of Abner Chang Sent: Thursday, February 4, 2021 4:36 PM To: devel@edk2.groups.io; dandan.bi@intel.com Cc: 'Dong, Eric' >; Wang, Nickle (HPS SW) > Subject: Re: [edk2-devel] [PATCH v3] MdeModulePkg/Library: Add HiiGetStringEx to UefiHiiLib for EDK2 Redfish This looks to me not quite intuitional. !(!TryBestLanguage && Language == NULL) seems more straightforward and exactly what we would like to catch. Abner Get Outlook for Android ________________________________ From: devel@edk2.groups.io > on behalf of gaoliming > Sent: Thursday, February 4, 2021, 3:13 PM To: devel@edk2.groups.io; Chang, Abner (HPS SW/FW Technologist); dandan.bi@intel.com Cc: 'Dong, Eric'; Wang, Nickle (HPS SW) Subject: 回复: [edk2-devel] [PATCH v3] MdeModulePkg/Library: Add HiiGetStringEx to UefiHiiLib for EDK2 Redfish If TryBestLanguage is FALE and Language is NULL, then ASSERT(). So, ASSERT statement should be ASSERT (TryBestLanguage || (Language != NULL)); Is this clear? Thanks Liming > -----邮件原件----- > 发件人: bounce+27952+71110+4905953+8761045@groups.io > > 代表 Abner Chang > 发送时间: 2021年2月3日 23:21 > 收件人: devel@edk2.groups.io; dandan.bi@intel.com > 抄送: Dong, Eric >; Wang, Nickle (HPS SW) > > > 主题: Re: [edk2-devel] [PATCH v3] MdeModulePkg/Library: Add > HiiGetStringEx to UefiHiiLib for EDK2 Redfish > > Yes Dandan, yours follow coding standard. I will handle that. > Also, I will help to push the patch to upstream with your review tag. > > Thanks > Abner > > > -----Original Message----- > > From: devel@edk2.groups.io [mailto:devel@edk2.groups.io] On Behalf Of > > Dandan Bi > > Sent: Wednesday, February 3, 2021 7:34 PM > > To: Chang, Abner (HPS SW/FW Technologist) >; > > devel@edk2.groups.io > > Cc: Dong, Eric >; Wang, Nickle (HPS SW) > > > > > Subject: Re: [edk2-devel] [PATCH v3] MdeModulePkg/Library: Add > > HiiGetStringEx to UefiHiiLib for EDK2 Redfish > > > > One minor comment inline below. > > Reviewed-by: Dandan Bi > with this is handled. > > > > Thanks, > > Dandan > > > -----Original Message----- > > > From: Abner Chang > > > > Sent: Monday, February 1, 2021 11:06 AM > > > To: devel@edk2.groups.io > > > Cc: Bi, Dandan >; Dong, Eric > > > >; Nickle Wang > > > > Subject: [PATCH v3] MdeModulePkg/Library: Add HiiGetStringEx to > > > UefiHiiLib for EDK2 Redfish > > > > > > Add HiiGetStringEx and leveraged by HiiGetString function to support > > > getting string with the best language in optionally. This avoids the > > > string in x-uefi language is misled to the language defined by > > > "PlatformLang" or the "Supported Languages". This change is introduced > > > to support x-uefi keyword language for configuring BIOS setting. > > > > > > Signed-off-by: Jiaxin Wu > > > > Signed-off-by: Siyuan Fu > > > > Signed-off-by: Fan Wang > > > > Signed-off-by: Abner Chang > > > > Cc: Dandan Bi > > > > Cc: Eric Dong > > > > Cc: Nickle Wang > > > > --- > > > MdeModulePkg/Include/Library/HiiLib.h | 60 ++++++++++--- > > > MdeModulePkg/Library/UefiHiiLib/HiiString.c | 98 > > > +++++++++++++++------ > > > 2 files changed, 119 insertions(+), 39 deletions(-) > > > > > > diff --git a/MdeModulePkg/Include/Library/HiiLib.h > > > b/MdeModulePkg/Include/Library/HiiLib.h > > > index c475cb74a1..cd9027fefd 100644 > > > --- a/MdeModulePkg/Include/Library/HiiLib.h > > > +++ b/MdeModulePkg/Include/Library/HiiLib.h > > > @@ -1,7 +1,8 @@ > > > /** @file > > > Public include file for the HII Library > > > > > > -Copyright (c) 2007 - 2018, Intel Corporation. All rights > > > reserved.
> > > +Copyright (c) 2007 - 2021, Intel Corporation. All rights > > > +reserved.
> > > +(C) Copyright 2021 Hewlett Packard Enterprise Development LP
> > > SPDX-License-Identifier: BSD-2-Clause-Patent > > > > > > **/ > > > @@ -123,16 +124,8 @@ HiiSetString ( > > > ; > > > > > > /** > > > - Retrieves a string from a string package in a specific language. > > > If the language > > > - is not specified, then a string from a string package in the > > > current platform > > > - language is retrieved. If the string cannot be retrieved using the > > > specified > > > - language or the current platform language, then the string is > > > retrieved from > > > - the string package in the first language the string package > > > supports. The > > > - returned string is allocated using AllocatePool(). The caller is > > > responsible > > > - for freeing the allocated buffer using FreePool(). > > > - > > > - If HiiHandle is NULL, then ASSERT(). > > > - If StringId is 0, then ASSERT(). > > > + Retrieves a string from a string package in a specific language > > > + specified in Language or in the best lanaguage. See HiiGetStringEx > > > + () for > > > the details. > > > > > > @param[in] HiiHandle A handle that was previously registered in > > > the HII Database. > > > @param[in] StringId The identifier of the string to retrieved from > the > > > string > > > @@ -152,8 +145,49 @@ HiiGetString ( > > > IN EFI_HII_HANDLE HiiHandle, > > > IN EFI_STRING_ID StringId, > > > IN CONST CHAR8 *Language OPTIONAL > > > - ) > > > -; > > > + ); > > > + > > > +/** > > > + Retrieves a string from a string package in a specific language or > > > +in the best > > > + language at discretion of this function according to the priority of > > languages. > > > + TryBestLanguage is used to get the string in the best language or > > > +in the language > > > + specified in Language parameter. The behavior is, > > > + If TryBestLanguage is TRUE, this function looks for the best > > > +language for > > > the string. > > > + - If the string can not be retrieved using the specified language > > > + or the > > > current > > > + platform language, then the string is retrieved from the string > > > + package in > > > the > > > + first language the string package supports. > > > + If TryBestLanguage is FALSE, Language must be specified for > > > + retrieving the > > > string. > > > + > > > + The returned string is allocated using AllocatePool(). The caller > > > + is responsible for freeing the allocated buffer using FreePool(). > > > + > > > + If HiiHandle is NULL, then ASSERT(). > > > + If StringId is 0, then ASSET. > > > + If TryBestLanguage is FALE and Language is NULL, then ASSERT(). > > > + > > > + @param[in] HiiHandle A handle that was previously > registered in the > > > HII Database. > > > + @param[in] StringId The identifier of the string to > retrieved from > > the > > > string > > > + package associated with > HiiHandle. > > > + @param[in] Language The language of the string to > retrieve. If this > > > parameter > > > + is NULL, then the current > platform language is used. The > > > + format of Language must follow > the > > > + language format > > > assumed > > > + the HII Database. > > > + @param[in] TryBestLanguage If TRUE, try to get the best > matching > > > language from all > > > + supported languages.If FALSE, > the > > > + Language must be > > > assigned > > > + for the StringID. > > > + > > > + @retval NULL The string specified by StringId is not present in the > string > > > package. > > > + @retval Other The string was returned. > > > + > > > +**/ > > > +EFI_STRING > > > +EFIAPI > > > +HiiGetStringEx ( > > > + IN EFI_HII_HANDLE HiiHandle, > > > + IN EFI_STRING_ID StringId, > > > + IN CONST CHAR8 *Language OPTIONAL, > > > + IN BOOLEAN TryBestLanguage > > > + ); > > > > > > /** > > > Retrieves a string from a string package named by GUID, in the > > > specified language. > > > diff --git a/MdeModulePkg/Library/UefiHiiLib/HiiString.c > > > b/MdeModulePkg/Library/UefiHiiLib/HiiString.c > > > index 95229f8a8c..a9a11aef98 100644 > > > --- a/MdeModulePkg/Library/UefiHiiLib/HiiString.c > > > +++ b/MdeModulePkg/Library/UefiHiiLib/HiiString.c > > > @@ -1,7 +1,8 @@ > > > /** @file > > > HII Library implementation that uses DXE protocols and services. > > > > > > - Copyright (c) 2006 - 2018, Intel Corporation. All rights > > > reserved.
> > > + Copyright (c) 2006 - 2021, Intel Corporation. All rights > > > + reserved.
> > > + (C) Copyright 2021 Hewlett Packard Enterprise Development LP
> > > SPDX-License-Identifier: BSD-2-Clause-Patent > > > > > > **/ > > > @@ -190,16 +191,8 @@ HiiGetPackageString ( } > > > > > > /** > > > - Retrieves a string from a string package in a specific language. > > > If the language > > > - is not specified, then a string from a string package in the > > > current platform > > > - language is retrieved. If the string can not be retrieved using > > > the specified > > > - language or the current platform language, then the string is > > > retrieved from > > > - the string package in the first language the string package > > > supports. The > > > - returned string is allocated using AllocatePool(). The caller is > > > responsible > > > - for freeing the allocated buffer using FreePool(). > > > - > > > - If HiiHandle is NULL, then ASSERT(). > > > - If StringId is 0, then ASSET. > > > + Retrieves a string from a string package in a specific language > > > + specified in Language or in the best lanaguage. See HiiGetStringEx > > > + () for > > > the details. > > > > > > @param[in] HiiHandle A handle that was previously registered in > > > the HII Database. > > > @param[in] StringId The identifier of the string to retrieved from > the > > > string > > > @@ -220,6 +213,51 @@ HiiGetString ( > > > IN EFI_STRING_ID StringId, > > > IN CONST CHAR8 *Language OPTIONAL > > > ) > > > +{ > > > + return HiiGetStringEx (HiiHandle, StringId, Language, TRUE); } > > > + > > > +/** > > > + Retrieves a string from a string package in a specific language or > > > +in the best > > > + language at discretion of this function according to the priority of > > languages. > > > + TryBestLanguage is used to get the string in the best language or > > > +in the language > > > + specified in Language parameter. The behavior is, > > > + If TryBestLanguage is TRUE, this function looks for the best > > > +language for > > > the string. > > > + - If the string can not be retrieved using the specified language > > > + or the > > > current > > > + platform language, then the string is retrieved from the string > > > + package in > > > the > > > + first language the string package supports. > > > + If TryBestLanguage is FALSE, Language must be specified for > > > + retrieving the > > > string. > > > + > > > + The returned string is allocated using AllocatePool(). The caller > > > + is responsible for freeing the allocated buffer using FreePool(). > > > + > > > + If HiiHandle is NULL, then ASSERT(). > > > + If StringId is 0, then ASSET. > > > + If TryBestLanguage is FALE and Language is NULL, then ASSERT(). > > > + > > > + @param[in] HiiHandle A handle that was previously > registered in the > > > HII Database. > > > + @param[in] StringId The identifier of the string to > retrieved from > > the > > > string > > > + package associated with > HiiHandle. > > > + @param[in] Language The language of the string to > retrieve. If this > > > parameter > > > + is NULL, then the current > platform language is used. The > > > + format of Language must follow > the > > > + language format > > > assumed > > > + the HII Database. > > > + @param[in] TryBestLanguage If TRUE, try to get the best > matching > > > language from all > > > + supported languages.If FALSE, > the > > > + Language must be > > > assigned > > > + for the StringID. > > > + > > > + @retval NULL The string specified by StringId is not present in the > string > > > package. > > > + @retval Other The string was returned. > > > + > > > +**/ > > > +EFI_STRING > > > +EFIAPI > > > +HiiGetStringEx ( > > > + IN EFI_HII_HANDLE HiiHandle, > > > + IN EFI_STRING_ID StringId, > > > + IN CONST CHAR8 *Language OPTIONAL, > > > + IN BOOLEAN TryBestLanguage > > > + ) > > > { > > > EFI_STATUS Status; > > > UINTN StringSize; > > > @@ -231,7 +269,10 @@ HiiGetString ( > > > > > > ASSERT (HiiHandle != NULL); > > > ASSERT (StringId != 0); > > > - > > > + // > > > + // Language must be specified if TryBestLanguage = FALSE. > > > + // > > > + ASSERT (!(TryBestLanguage == FALSE && Language == NULL)); > > One minor here, in order to following the coding style, this seems should be > > ASSERT (!(!TryBestLanguage && Language == NULL)); Please help double > > check. > > > > > // > > > // Initialize all allocated buffers to NULL > > > // > > > @@ -261,21 +302,26 @@ HiiGetString ( > > > Language = ""; > > > } > > > > > > - // > > > - // Get the best matching language from SupportedLanguages > > > - // > > > - BestLanguage = GetBestLanguage ( > > > - SupportedLanguages, > > > - FALSE, > // RFC 4646 mode > > > - Language, > // Highest priority > > > - PlatformLanguage != NULL ? PlatformLanguage : > "", // Next > > > highest priority > > > - SupportedLanguages, > // Lowest priority > > > - NULL > > > - ); > > > - if (BestLanguage == NULL) { > > > - goto Error; > > > + if (TryBestLanguage) { > > > + // > > > + // Get the best matching language from SupportedLanguages > > > + // > > > + BestLanguage = GetBestLanguage ( > > > + SupportedLanguages, > > > + FALSE, > // RFC 4646 mode > > > + Language, > // Highest priority > > > + PlatformLanguage != NULL ? > PlatformLanguage : > > > + "", // Next > > > highest priority > > > + SupportedLanguages, > // Lowest priority > > > + NULL > > > + ); > > > + if (BestLanguage == NULL) { > > > + goto Error; > > > + } > > > + } else { > > > + BestLanguage = (CHAR8 *) Language; > > > } > > > > > > + > > > // > > > // Retrieve the size of the string in the string package for the > > BestLanguage > > > // > > > @@ -337,7 +383,7 @@ Error: > > > if (PlatformLanguage != NULL) { > > > FreePool (PlatformLanguage); > > > } > > > - if (BestLanguage != NULL) { > > > + if (TryBestLanguage && BestLanguage != NULL) { > > > FreePool (BestLanguage); > > > } > > > > > > -- > > > 2.17.1 > > > > > > > > > > > > > > >