Tom: I miss one minor comments on doxygen style in function header comments. Please see below. The code logic is good to me. Reviewed-by: Liming Gao <liming....@intel.com>
> -----Original Message----- > From: devel@edk2.groups.io [mailto:devel@edk2.groups.io] On Behalf Of Tom Zhao > Sent: Wednesday, September 11, 2019 7:06 PM > To: devel@edk2.groups.io > Cc: Kinney, Michael D <michael.d.kin...@intel.com>; Gao, Liming > <liming....@intel.com> > Subject: [edk2-devel] [PATCH v4 1/1] MdePkg: UefiLib: Add a function to check > if a language is supported > > Add a function that checks if a target language is in the supported > languages list. Add some calls to this function where appropriate in > UefiLib.c > > Cc: Michael D Kinney <michael.d.kin...@intel.com> > Cc: Liming Gao <liming....@intel.com> > Signed-off-by: Tom Zhao <tz...@solarflare.com> > --- > > Notes: > v3: > - Add comment about usage for RFC4646 and ISO-639 > v4: > - Remove reference to CompareISO639Lang > > MdePkg/Include/Library/UefiLib.h | 17 ++++++ > MdePkg/Library/UefiLib/UefiLib.c | 60 +++++++++++++------- > 2 files changed, 56 insertions(+), 21 deletions(-) > > diff --git a/MdePkg/Include/Library/UefiLib.h > b/MdePkg/Include/Library/UefiLib.h > index 1650f30ddbc6..67c6f96747ca 100644 > --- a/MdePkg/Include/Library/UefiLib.h > +++ b/MdePkg/Include/Library/UefiLib.h > @@ -461,6 +461,23 @@ EfiTestChildHandle ( > IN CONST EFI_GUID *ProtocolGuid > ); > +/** > + * This function checks the supported languages list for a target language, > + * This only supports RFC 4646 Languages. > + * > + * @param SupportedLanguages The supported languages > + * @param TargetLanguage The target language > + * > + * @return Returns EFI_SUCCESS if the language is supported, > + * EFI_UNSUPPORTED otherwise Here should @retval EFI_SUCCESS The target language is supported @retval EFI_UNSUPPORTED The target language is unsupported > + */ > +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..cc03be84c2d8 100644 > --- a/MdePkg/Library/UefiLib/UefiLib.c > +++ b/MdePkg/Library/UefiLib/UefiLib.c > @@ -640,6 +640,36 @@ EfiTestChildHandle ( > return Status; > } > +/** > + * This function checks the supported languages list for a target language, > + * This only supports RFC 4646 Languages. > + * > + * @param SupportedLanguages The supported languages > + * @param TargetLanguage The target language > + * > + * @return Returns EFI_SUCCESS if the language is supported, > + * EFI_UNSUPPORTED otherwise Here should be @retval EFI_SUCCESS The target language is supported @retval EFI_UNSUPPORTED The target language is unsupported Thanks Liming > + */ > +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 +830,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 +1124,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.16.5 > > > -=-=-=-=-=-=-=-=-=-=-=- Groups.io Links: You receive all messages sent to this group. View/Reply Online (#47122): https://edk2.groups.io/g/devel/message/47122 Mute This Topic: https://groups.io/mt/34102079/21656 Group Owner: devel+ow...@edk2.groups.io Unsubscribe: https://edk2.groups.io/g/devel/unsub [arch...@mail-archive.com] -=-=-=-=-=-=-=-=-=-=-=-