Re: [PATCH v4 2/7] MdePkg/BaseLib: Introduce CharToUpper and AsciiToUpper publicly

["Ni, Ruiyu" <ruiyu.ni@Intel.com>]

On 12/25/2018 4:17 PM, Shenglei Zhang wrote:
> Introduce two public functions CharToUpper and AsciiToUpper.
> They have the same functions as InternalCharToUpper and
> InternalBaseLibAsciiToUpper.Considering the internal functions will
> be removed,so directly I change their function names to the public ones'.
> https://bugzilla.tianocore.org/show_bug.cgi?id=1369
> 
> Cc: Leif Lindholm <leif.lindholm@linaro.org>
> Cc: Laszlo Ersek <lersek@redhat.com>
> Cc: Michael D Kinney <michael.d.kinney@intel.com>
> Cc: Liming Gao <liming.gao@intel.com>
> Contributed-under: TianoCore Contribution Agreement 1.1
> Signed-off-by: Shenglei Zhang <shenglei.zhang@intel.com>
> ---
>   MdePkg/Include/Library/BaseLib.h    | 40 +++++++++++++++++++++++++++++
>   MdePkg/Library/BaseLib/SafeString.c |  8 +++---
>   MdePkg/Library/BaseLib/String.c     | 16 ++++++------
>   3 files changed, 52 insertions(+), 12 deletions(-)
> 
> diff --git a/MdePkg/Include/Library/BaseLib.h b/MdePkg/Include/Library/BaseLib.h
> index 1eb842384e..e8cc121ddd 100644
> --- a/MdePkg/Include/Library/BaseLib.h
> +++ b/MdePkg/Include/Library/BaseLib.h
> @@ -2720,6 +2720,46 @@ AsciiStrnToUnicodeStrS (
>     OUT     UINTN                     *DestinationLength
>     );
>   
> +/**
> +  Convert a Unicode character to upper case only if
> +  it maps to a valid small-case ASCII character.
> +
> +  This internal function only deal with Unicode character
> +  which maps to a valid small-case ASCII character, i.e.
> +  L'a' to L'z'. For other Unicode character, the input character
> +  is returned directly.
> +
> +  @param  Char  The character to convert.
> +
> +  @retval LowerCharacter   If the Char is with range L'a' to L'z'.
> +  @retval Unchanged        Otherwise.
> +
> +**/
> +CHAR16
> +EFIAPI
> +CharToUpper (
> +  IN      CHAR16                    Char
> +  );
> +
> +/**
> +  Converts a lowercase Ascii character to upper one.
> +
> +  If Chr is lowercase Ascii character, then converts it to upper one.
> +
> +  If Value >= 0xA0, then ASSERT().
> +  If (Value & 0x0F) >= 0x0A, then ASSERT().
> +
> +  @param  Chr   one Ascii character
> +
> +  @return The uppercase value of Ascii character
> +
> +**/
> +CHAR8
> +EFIAPI
> +AsciiToUpper (
> +  IN      CHAR8                     Chr
> +  );
> +
>   /**
>     Converts an 8-bit value to an 8-bit BCD value.
>   
> diff --git a/MdePkg/Library/BaseLib/SafeString.c b/MdePkg/Library/BaseLib/SafeString.c
> index 417497cbc9..17f88b46d8 100644
> --- a/MdePkg/Library/BaseLib/SafeString.c
> +++ b/MdePkg/Library/BaseLib/SafeString.c
> @@ -905,7 +905,7 @@ StrHexToUintnS (
>       String++;
>     }
>   
> -  if (InternalCharToUpper (*String) == L'X') {
> +  if (CharToUpper (*String) == L'X') {
>       if (*(String - 1) != L'0') {
>         *Data = 0;
>         return RETURN_SUCCESS;
> @@ -1036,7 +1036,7 @@ StrHexToUint64S (
>       String++;
>     }
>   
> -  if (InternalCharToUpper (*String) == L'X') {
> +  if (CharToUpper (*String) == L'X') {
>       if (*(String - 1) != L'0') {
>         *Data = 0;
>         return RETURN_SUCCESS;
> @@ -2459,7 +2459,7 @@ AsciiStrHexToUintnS (
>       String++;
>     }
>   
> -  if (InternalBaseLibAsciiToUpper (*String) == 'X') {
> +  if (AsciiToUpper (*String) == 'X') {
>       if (*(String - 1) != '0') {
>         *Data = 0;
>         return RETURN_SUCCESS;
> @@ -2586,7 +2586,7 @@ AsciiStrHexToUint64S (
>       String++;
>     }
>   
> -  if (InternalBaseLibAsciiToUpper (*String) == 'X') {
> +  if (AsciiToUpper (*String) == 'X') {
>       if (*(String - 1) != '0') {
>         *Data = 0;
>         return RETURN_SUCCESS;
> diff --git a/MdePkg/Library/BaseLib/String.c b/MdePkg/Library/BaseLib/String.c
> index e6df12797d..dba53779c9 100644
> --- a/MdePkg/Library/BaseLib/String.c
> +++ b/MdePkg/Library/BaseLib/String.c
> @@ -552,7 +552,7 @@ InternalIsDecimalDigitCharacter (
>   **/
>   CHAR16
>   EFIAPI
> -InternalCharToUpper (
> +CharToUpper (
>     IN      CHAR16                    Char
>     )
>   {
> @@ -586,7 +586,7 @@ InternalHexCharToUintn (
>       return Char - L'0';
>     }
>   
> -  return (10 + InternalCharToUpper (Char) - L'A');
> +  return (10 + CharToUpper (Char) - L'A');
>   }
>   
>   /**
> @@ -1181,7 +1181,7 @@ AsciiStrCmp (
>   **/
>   CHAR8
>   EFIAPI
> -InternalBaseLibAsciiToUpper (
> +AsciiToUpper (
>     IN      CHAR8                     Chr
>     )
>   {
> @@ -1211,7 +1211,7 @@ InternalAsciiHexCharToUintn (
>       return Char - '0';
>     }
>   
> -  return (10 + InternalBaseLibAsciiToUpper (Char) - 'A');
> +  return (10 + AsciiToUpper (Char) - 'A');
>   }
>   
>   
> @@ -1260,13 +1260,13 @@ AsciiStriCmp (
>     ASSERT (AsciiStrSize (FirstString));
>     ASSERT (AsciiStrSize (SecondString));
>   
> -  UpperFirstString  = InternalBaseLibAsciiToUpper (*FirstString);
> -  UpperSecondString = InternalBaseLibAsciiToUpper (*SecondString);
> +  UpperFirstString  = AsciiToUpper (*FirstString);
> +  UpperSecondString = AsciiToUpper (*SecondString);
>     while ((*FirstString != '\0') && (*SecondString != '\0') && (UpperFirstString == UpperSecondString)) {
>       FirstString++;
>       SecondString++;
> -    UpperFirstString  = InternalBaseLibAsciiToUpper (*FirstString);
> -    UpperSecondString = InternalBaseLibAsciiToUpper (*SecondString);
> +    UpperFirstString  = AsciiToUpper (*FirstString);
> +    UpperSecondString = AsciiToUpper (*SecondString);
>     }
>   
>     return UpperFirstString - UpperSecondString;
> 

Shenglei,
Do we need to make the two API names consistent to existing string APIs
in BaseLib?
All ASCII version of string APIs have the "Ascii" prefix, for example:
StrLen, AsciiStrLen
StrHexToUintn, AsciiStrHexToUintn.

So how about to use "CharToUpper" and "AsciiCharToUpper"?

By the way, I cannot find the implementation of CharToUpper() in your patch.

-- 
Thanks,
Ray