* Re: [edk2-devel] [PATCH v2 1/1] MdePkg: UefiLib: Add a function to check if a language is supported
2019-09-02 3:13 ` [edk2-devel] " Liming Gao
@ 2019-09-02 8:27 ` Tom Zhao
2019-09-02 9:35 ` Tomas Pilar (tpilar)
1 sibling, 0 replies; 4+ messages in thread
From: Tom Zhao @ 2019-09-02 8:27 UTC (permalink / raw)
To: devel, liming.gao; +Cc: Kinney, Michael D
Hi Liming
The IsLanguageSupported() is intended for use in implementations of UEFI
Driver Model protocols. The protocol functions that will likely use this
are ones where the specification requires a language check, such as
EFI_DRIVER_DIAGNOSTIC2_PROTOCOL.RunDiagnostics(). Currently, to check if
a language is supported using the API we must do extra work and retrieve
the actual string, however, in many cases this string would just be
thrown away later.
Tom
On 02/09/2019 04:13, Liming Gao wrote:
> Tom:
> New API IsLanguageSupported() only supports RFC 4646 language code for the Unicode string.
> If you think ISO 639-2 language code is obsolete, and don't plan to support it any longer. Please
> update IsLanguageSupported() function description to clarify this API for RFC 4646 language code.
>
> And, I still want to know what other usage model for this new API IsLanguageSupported(). For example,
> how UEFI HII driver consumes this API? If this API is only used in UefiLib, it can be the internal function
> in UefiLib library instance.
>
> Thanks
> Liming
>> -----Original Message-----
>> From: devel@edk2.groups.io [mailto:devel@edk2.groups.io] On Behalf Of Tom Zhao
>> Sent: Friday, August 30, 2019 7:34 PM
>> To: devel@edk2.groups.io
>> Cc: Gao, Liming <liming.gao@intel.com>; Kinney, Michael D <michael.d.kinney@intel.com>
>> Subject: [edk2-devel] [PATCH v2 1/1] MdePkg: UefiLib: Add a function to check if a language is supported
>>
>> BZ:https://bugzilla.tianocore.org/show_bug.cgi?id=2100
>>
>> Add a function that checks if a target language is in the supported
>> languages list. Refactor UefiLib to use this function where appropriate
>> internally.
>>
>> Cc: Michael D Kinney <michael.d.kinney@intel.com>
>> Cc: Liming Gao <liming.gao@intel.com>
>> Signed-off-by: Tom Zhao <tzhao@solarflare.com>
>> ---
>> MdePkg/Include/Library/UefiLib.h | 16 ++++++
>> MdePkg/Library/UefiLib/UefiLib.c | 59 +++++++++++++-------
>> 2 files changed, 54 insertions(+), 21 deletions(-)
>>
>> diff --git a/MdePkg/Include/Library/UefiLib.h
>> b/MdePkg/Include/Library/UefiLib.h
>> index 1650f30ddbc6..9dd170cbe2bf 100644
>> --- a/MdePkg/Include/Library/UefiLib.h
>> +++ b/MdePkg/Include/Library/UefiLib.h
>> @@ -461,6 +461,22 @@ EfiTestChildHandle (
>> IN CONST EFI_GUID *ProtocolGuid
>> );
>> +/**
>> + * This function checks the supported languages list for a target language
>> + *
>> + * @param SupportedLanguages The supported languages
>> + * @param TargetLanguage The target language
>> + *
>> + * @return Returns EFI_SUCCESS if the language is supported,
>> + * EFI_UNSUPPORTED otherwise
>> + */
>> +EFI_STATUS
>> +EFIAPI
>> +IsLanguageSupported (
>> + IN CONST CHAR8 *SupportedLanguages,
>> + IN CONST CHAR8 *TargetLanguage
>> + );
>> +
>> /**
>> This function looks up a Unicode string in UnicodeStringTable.
>> diff --git a/MdePkg/Library/UefiLib/UefiLib.c
>> b/MdePkg/Library/UefiLib/UefiLib.c
>> index daa4af762e62..56281d25fd99 100644
>> --- a/MdePkg/Library/UefiLib/UefiLib.c
>> +++ b/MdePkg/Library/UefiLib/UefiLib.c
>> @@ -640,6 +640,35 @@ EfiTestChildHandle (
>> return Status;
>> }
>> +/**
>> + * This function checks the supported languages list for a target language
>> + *
>> + * @param SupportedLanguages The supported languages
>> + * @param TargetLanguage The target language
>> + *
>> + * @return Returns EFI_SUCCESS if the language is supported,
>> + * EFI_UNSUPPORTED otherwise
>> + */
>> +EFI_STATUS
>> +EFIAPI
>> +IsLanguageSupported (
>> + IN CONST CHAR8 *SupportedLanguages,
>> + IN CONST CHAR8 *TargetLanguage
>> + )
>> +{
>> + UINTN Index;
>> + while (*SupportedLanguages != 0) {
>> + for (Index = 0; SupportedLanguages[Index] != 0 &&
>> SupportedLanguages[Index] != ';'; Index++);
>> + if ((AsciiStrnCmp(SupportedLanguages, TargetLanguage, Index) == 0)
>> && (TargetLanguage[Index] == 0)) {
>> + return EFI_SUCCESS;
>> + }
>> + SupportedLanguages += Index;
>> + for (; *SupportedLanguages != 0 && *SupportedLanguages == ';';
>> SupportedLanguages++);
>> + }
>> +
>> + return EFI_UNSUPPORTED;
>> +}
>> +
>> /**
>> This function looks up a Unicode string in UnicodeStringTable.
>> @@ -800,24 +829,19 @@ LookupUnicodeString2 (
>> // Make sure Language is in the set of Supported Languages
>> //
>> Found = FALSE;
>> - while (*SupportedLanguages != 0) {
>> - if (Iso639Language) {
>> + if (Iso639Language) {
>> + while (*SupportedLanguages != 0) {
>> if (CompareIso639LanguageCode (Language, SupportedLanguages)) {
>> Found = TRUE;
>> break;
>> }
>> SupportedLanguages += 3;
>> - } else {
>> - for (Index = 0; SupportedLanguages[Index] != 0 &&
>> SupportedLanguages[Index] != ';'; Index++);
>> - if ((AsciiStrnCmp(SupportedLanguages, Language, Index) == 0) &&
>> (Language[Index] == 0)) {
>> - Found = TRUE;
>> - break;
>> - }
>> - SupportedLanguages += Index;
>> - for (; *SupportedLanguages != 0 && *SupportedLanguages == ';';
>> SupportedLanguages++);
>> }
>> + } else {
>> + Found = !IsLanguageSupported(Language, SupportedLanguages);
>> }
>> +
>> //
>> // If Language is not a member of SupportedLanguages, then return
>> EFI_UNSUPPORTED
>> //
>> @@ -1099,24 +1123,17 @@ AddUnicodeString2 (
>> // Make sure Language is a member of SupportedLanguages
>> //
>> Found = FALSE;
>> - while (*SupportedLanguages != 0) {
>> - if (Iso639Language) {
>> + if (Iso639Language) {
>> + while (*SupportedLanguages != 0) {
>> if (CompareIso639LanguageCode (Language, SupportedLanguages)) {
>> Found = TRUE;
>> break;
>> }
>> SupportedLanguages += 3;
>> - } else {
>> - for (Index = 0; SupportedLanguages[Index] != 0 &&
>> SupportedLanguages[Index] != ';'; Index++);
>> - if (AsciiStrnCmp(SupportedLanguages, Language, Index) == 0) {
>> - Found = TRUE;
>> - break;
>> - }
>> - SupportedLanguages += Index;
>> - for (; *SupportedLanguages != 0 && *SupportedLanguages == ';';
>> SupportedLanguages++);
>> }
>> + } else {
>> + Found = !IsLanguageSupported(Language, SupportedLanguages);
>> }
>> -
>> //
>> // If Language is not a member of SupportedLanguages, then return
>> EFI_UNSUPPORTED
>> //
>> --
>> 2.21.0
>>
>>
>>
>
>
>
^ permalink raw reply [flat|nested] 4+ messages in thread* Re: [edk2-devel] [PATCH v2 1/1] MdePkg: UefiLib: Add a function to check if a language is supported
2019-09-02 3:13 ` [edk2-devel] " Liming Gao
2019-09-02 8:27 ` Tom Zhao
@ 2019-09-02 9:35 ` Tomas Pilar (tpilar)
1 sibling, 0 replies; 4+ messages in thread
From: Tomas Pilar (tpilar) @ 2019-09-02 9:35 UTC (permalink / raw)
To: Devel EDK2, Tom Zhao, liming.gao@intel.com; +Cc: Kinney, Michael D
Hi Liming,
I think the very general idea is that UEFI spec requires language based DriverModel protocol members
to always check if the language is supported and return EFI_UNSUPPORTED otherwise. The procotols
such as ComponentName2, DriverDiagnostics2, etc. usually also publish a member string denoting
supported languages.
Therefore, a method that takes input language and checks against supported languages is going to be
quite popular in these functions.
The refactor in UefiLib is merely incidental really.
Cheers,
(other) Tom
________________________________________
From: devel@edk2.groups.io <devel@edk2.groups.io> on behalf of Liming Gao <liming.gao@intel.com>
Sent: 02 September 2019 04:13
To: Devel EDK2; Tom Zhao
Cc: Kinney, Michael D
Subject: Re: [edk2-devel] [PATCH v2 1/1] MdePkg: UefiLib: Add a function to check if a language is supported
Tom:
New API IsLanguageSupported() only supports RFC 4646 language code for the Unicode string.
If you think ISO 639-2 language code is obsolete, and don't plan to support it any longer. Please
update IsLanguageSupported() function description to clarify this API for RFC 4646 language code.
And, I still want to know what other usage model for this new API IsLanguageSupported(). For example,
how UEFI HII driver consumes this API? If this API is only used in UefiLib, it can be the internal function
in UefiLib library instance.
Thanks
Liming
> -----Original Message-----
> From: devel@edk2.groups.io [mailto:devel@edk2.groups.io] On Behalf Of Tom Zhao
> Sent: Friday, August 30, 2019 7:34 PM
> To: devel@edk2.groups.io
> Cc: Gao, Liming <liming.gao@intel.com>; Kinney, Michael D <michael.d.kinney@intel.com>
> Subject: [edk2-devel] [PATCH v2 1/1] MdePkg: UefiLib: Add a function to check if a language is supported
>
> BZ:https://bugzilla.tianocore.org/show_bug.cgi?id=2100
>
> Add a function that checks if a target language is in the supported
> languages list. Refactor UefiLib to use this function where appropriate
> internally.
>
> Cc: Michael D Kinney <michael.d.kinney@intel.com>
> Cc: Liming Gao <liming.gao@intel.com>
> Signed-off-by: Tom Zhao <tzhao@solarflare.com>
> ---
> MdePkg/Include/Library/UefiLib.h | 16 ++++++
> MdePkg/Library/UefiLib/UefiLib.c | 59 +++++++++++++-------
> 2 files changed, 54 insertions(+), 21 deletions(-)
>
> diff --git a/MdePkg/Include/Library/UefiLib.h
> b/MdePkg/Include/Library/UefiLib.h
> index 1650f30ddbc6..9dd170cbe2bf 100644
> --- a/MdePkg/Include/Library/UefiLib.h
> +++ b/MdePkg/Include/Library/UefiLib.h
> @@ -461,6 +461,22 @@ EfiTestChildHandle (
> IN CONST EFI_GUID *ProtocolGuid
> );
> +/**
> + * This function checks the supported languages list for a target language
> + *
> + * @param SupportedLanguages The supported languages
> + * @param TargetLanguage The target language
> + *
> + * @return Returns EFI_SUCCESS if the language is supported,
> + * EFI_UNSUPPORTED otherwise
> + */
> +EFI_STATUS
> +EFIAPI
> +IsLanguageSupported (
> + IN CONST CHAR8 *SupportedLanguages,
> + IN CONST CHAR8 *TargetLanguage
> + );
> +
> /**
> This function looks up a Unicode string in UnicodeStringTable.
> diff --git a/MdePkg/Library/UefiLib/UefiLib.c
> b/MdePkg/Library/UefiLib/UefiLib.c
> index daa4af762e62..56281d25fd99 100644
> --- a/MdePkg/Library/UefiLib/UefiLib.c
> +++ b/MdePkg/Library/UefiLib/UefiLib.c
> @@ -640,6 +640,35 @@ EfiTestChildHandle (
> return Status;
> }
> +/**
> + * This function checks the supported languages list for a target language
> + *
> + * @param SupportedLanguages The supported languages
> + * @param TargetLanguage The target language
> + *
> + * @return Returns EFI_SUCCESS if the language is supported,
> + * EFI_UNSUPPORTED otherwise
> + */
> +EFI_STATUS
> +EFIAPI
> +IsLanguageSupported (
> + IN CONST CHAR8 *SupportedLanguages,
> + IN CONST CHAR8 *TargetLanguage
> + )
> +{
> + UINTN Index;
> + while (*SupportedLanguages != 0) {
> + for (Index = 0; SupportedLanguages[Index] != 0 &&
> SupportedLanguages[Index] != ';'; Index++);
> + if ((AsciiStrnCmp(SupportedLanguages, TargetLanguage, Index) == 0)
> && (TargetLanguage[Index] == 0)) {
> + return EFI_SUCCESS;
> + }
> + SupportedLanguages += Index;
> + for (; *SupportedLanguages != 0 && *SupportedLanguages == ';';
> SupportedLanguages++);
> + }
> +
> + return EFI_UNSUPPORTED;
> +}
> +
> /**
> This function looks up a Unicode string in UnicodeStringTable.
> @@ -800,24 +829,19 @@ LookupUnicodeString2 (
> // Make sure Language is in the set of Supported Languages
> //
> Found = FALSE;
> - while (*SupportedLanguages != 0) {
> - if (Iso639Language) {
> + if (Iso639Language) {
> + while (*SupportedLanguages != 0) {
> if (CompareIso639LanguageCode (Language, SupportedLanguages)) {
> Found = TRUE;
> break;
> }
> SupportedLanguages += 3;
> - } else {
> - for (Index = 0; SupportedLanguages[Index] != 0 &&
> SupportedLanguages[Index] != ';'; Index++);
> - if ((AsciiStrnCmp(SupportedLanguages, Language, Index) == 0) &&
> (Language[Index] == 0)) {
> - Found = TRUE;
> - break;
> - }
> - SupportedLanguages += Index;
> - for (; *SupportedLanguages != 0 && *SupportedLanguages == ';';
> SupportedLanguages++);
> }
> + } else {
> + Found = !IsLanguageSupported(Language, SupportedLanguages);
> }
> +
> //
> // If Language is not a member of SupportedLanguages, then return
> EFI_UNSUPPORTED
> //
> @@ -1099,24 +1123,17 @@ AddUnicodeString2 (
> // Make sure Language is a member of SupportedLanguages
> //
> Found = FALSE;
> - while (*SupportedLanguages != 0) {
> - if (Iso639Language) {
> + if (Iso639Language) {
> + while (*SupportedLanguages != 0) {
> if (CompareIso639LanguageCode (Language, SupportedLanguages)) {
> Found = TRUE;
> break;
> }
> SupportedLanguages += 3;
> - } else {
> - for (Index = 0; SupportedLanguages[Index] != 0 &&
> SupportedLanguages[Index] != ';'; Index++);
> - if (AsciiStrnCmp(SupportedLanguages, Language, Index) == 0) {
> - Found = TRUE;
> - break;
> - }
> - SupportedLanguages += Index;
> - for (; *SupportedLanguages != 0 && *SupportedLanguages == ';';
> SupportedLanguages++);
> }
> + } else {
> + Found = !IsLanguageSupported(Language, SupportedLanguages);
> }
> -
> //
> // If Language is not a member of SupportedLanguages, then return
> EFI_UNSUPPORTED
> //
> --
> 2.21.0
>
>
>
^ permalink raw reply [flat|nested] 4+ messages in thread