[PATCH v3 0/3] Introduce two public functions and remove internal ones

[PATCH v3 0/3] Introduce two public functions and remove internal ones

[Shenglei Zhang]

Introduce public functions CharToUpper and AsciiToUpper.
Remove internal functions InternalCharToUpper and InternalBaseLibAsciiToUpper.

v2: Update the title and commit message of 1/3.

v3: 1.Add a patch to remove AsciiToUpper in EdbSupportString.c.
    2.Revert the changes in Basetools.

Cc: Michael D Kinney <michael.d.kinney@intel.com>
Cc: Liming Gao <liming.gao@intel.com>
Cc: Leif Lindholm <leif.lindholm@linaro.org>
Cc: Laszlo Ersek <lersek@redhat.com>
Cc: Jian J Wang <jian.j.wang@intel.com>
Cc: Hao Wu <hao.a.wu@intel.com>
Cc: Star Zeng <star.zeng@intel.com>
Shenglei Zhang (3):
  MdePkg/BaseLib: Introduce CharToUpper and AsciiToUpper publicly
  MdePkg/BaseLib: Remove InternalCharToUpper and
    InternalBaseLibAsciiToUpper
  MdeModulePkg/EbcDebugger: Remove an internal function AsciiToUpper

 .../EbcDxe/EbcDebugger/EdbSupportString.c     | 15 -------
 MdePkg/Include/Library/BaseLib.h              | 40 ++++++++++++++++++
 MdePkg/Library/BaseLib/BaseLibInternals.h     | 42 -------------------
 MdePkg/Library/BaseLib/SafeString.c           |  8 ++--
 MdePkg/Library/BaseLib/String.c               | 16 +++----
 5 files changed, 52 insertions(+), 69 deletions(-)

-- 
2.18.0.windows.1

[PATCH v3 1/3] MdePkg/BaseLib: Introduce CharToUpper and AsciiToUpper publicly

[Shenglei Zhang]

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 8cc086983d..b861d82287 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;
-- 
2.18.0.windows.1

[PATCH v3 2/3] MdePkg/BaseLib: Remove InternalCharToUpper and InternalBaseLibAsciiToUpper

[Shenglei Zhang]

InternalCharToUpper and InternalBaseLibAsciiToUpper are internal functions
and now we will introduce public functions that have the same effects.
So I remove their definition in BaseLibInternals.h.
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/Library/BaseLib/BaseLibInternals.h | 42 -----------------------
 1 file changed, 42 deletions(-)

diff --git a/MdePkg/Library/BaseLib/BaseLibInternals.h b/MdePkg/Library/BaseLib/BaseLibInternals.h
index 8855231c1a..9db925b157 100644
--- a/MdePkg/Library/BaseLib/BaseLibInternals.h
+++ b/MdePkg/Library/BaseLib/BaseLibInternals.h
@@ -469,28 +469,6 @@ InternalIsDecimalDigitCharacter (
   );
 
 
-/**
-  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
-InternalCharToUpper (
-  IN      CHAR16                    Char
-  );
-
-
 /**
   Convert a Unicode character to numerical value.
 
@@ -552,26 +530,6 @@ InternalAsciiIsDecimalDigitCharacter (
   );
 
 
-/**
-  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
-InternalBaseLibAsciiToUpper (
-  IN      CHAR8                     Chr
-  );
-
-
 /**
   Check if a ASCII character is a hexadecimal character.
 
-- 
2.18.0.windows.1

[PATCH v3 3/3] MdeModulePkg/EbcDebugger: Remove an internal function AsciiToUpper

[Shenglei Zhang]

AsciiToUpper in EdbSupportString.c is an internal function.
And we will introduce a public function which has the same name
and effect. So we remove its implement. Code in EdbSupportString.c
now consumes the public one.
https://bugzilla.tianocore.org/show_bug.cgi?id=1369

Cc: Jian J Wang <jian.j.wang@intel.com>
Cc: Hao Wu <hao.a.wu@intel.com>
Cc: Star Zeng <star.zeng@intel.com>
Contributed-under: TianoCore Contribution Agreement 1.1
Signed-off-by: Shenglei Zhang <shenglei.zhang@intel.com>
---
 .../EbcDxe/EbcDebugger/EdbSupportString.c         | 15 ---------------
 1 file changed, 15 deletions(-)

diff --git a/MdeModulePkg/Universal/EbcDxe/EbcDebugger/EdbSupportString.c b/MdeModulePkg/Universal/EbcDxe/EbcDebugger/EdbSupportString.c
index 78a0559079..0eb894ddfb 100644
--- a/MdeModulePkg/Universal/EbcDxe/EbcDebugger/EdbSupportString.c
+++ b/MdeModulePkg/Universal/EbcDxe/EbcDebugger/EdbSupportString.c
@@ -323,21 +323,6 @@ UnicodeToUpper (
   return (Chr >= L'a' && Chr <= L'z') ? Chr - (L'a' - L'A') : Chr;
 }
 
-/**
-
-  Convert the character to upper case.
-
-  @param  Chr    the character to be converted.
-
-**/
-STATIC
-CHAR8
-AsciiToUpper (
-  IN      CHAR8                     Chr
-  )
-{
-  return (Chr >= 'a' && Chr <= 'z') ? Chr - ('a' - 'A') : Chr;
-}
 
 /**
   Compare the Unicode and Ascii string pointed by String to the string pointed by String2.
-- 
2.18.0.windows.1

Re: [PATCH v3 3/3] MdeModulePkg/EbcDebugger: Remove an internal function AsciiToUpper

[Wu, Hao A]

Hi,

I think the patch will bring issue with regard to bisecting.

One way to resolve this is to introduce a patch to rename the 'AsciiToUpper'
within file EdbSupportString.c to something like 'InternalAsciiToUpper'. And
make the new patch as the 1st one of the series. Then goes the remaining 3
patches of this version of the series.


I also have a question that will we perform a cleanup for other 'XXXToUpper'
functions within the code base?

I did a quick search for keyword 'ToUpper', and found some (not all) possible
candidates that can be replaced with the newly added BaseLib APIs:

FatPkg\FatPei\FatLiteLib.c
Line 34: ToUpper (

MdeModulePkg\Library\DxeHttpLib\DxeHttpLib.c
Line 911: HttpIoCharToUpper (

MdeModulePkg\Universal\EbcDxe\EbcDebugger\EdbSupportString.c
Line 319: UnicodeToUpper (

ShellPkg\Library\UefiShellDebug1CommandsLib\UefiShellDebug1CommandsLib.c
Line 134: CharToUpper (

ShellPkg\Library\UefiShellLib\UefiShellLib.c
Line 3759: InternalShellCharToUpper (


Best Regards,
Hao Wu

> -----Original Message-----
> From: Zhang, Shenglei
> Sent: Wednesday, December 19, 2018 4:43 PM
> To: edk2-devel@lists.01.org
> Cc: Wang, Jian J; Wu, Hao A; Zeng, Star
> Subject: [PATCH v3 3/3] MdeModulePkg/EbcDebugger: Remove an internal
> function AsciiToUpper
> 
> AsciiToUpper in EdbSupportString.c is an internal function.
> And we will introduce a public function which has the same name
> and effect. So we remove its implement. Code in EdbSupportString.c
> now consumes the public one.
> https://bugzilla.tianocore.org/show_bug.cgi?id=1369
> 
> Cc: Jian J Wang <jian.j.wang@intel.com>
> Cc: Hao Wu <hao.a.wu@intel.com>
> Cc: Star Zeng <star.zeng@intel.com>
> Contributed-under: TianoCore Contribution Agreement 1.1
> Signed-off-by: Shenglei Zhang <shenglei.zhang@intel.com>
> ---
>  .../EbcDxe/EbcDebugger/EdbSupportString.c         | 15 ---------------
>  1 file changed, 15 deletions(-)
> 
> diff --git
> a/MdeModulePkg/Universal/EbcDxe/EbcDebugger/EdbSupportString.c
> b/MdeModulePkg/Universal/EbcDxe/EbcDebugger/EdbSupportString.c
> index 78a0559079..0eb894ddfb 100644
> --- a/MdeModulePkg/Universal/EbcDxe/EbcDebugger/EdbSupportString.c
> +++ b/MdeModulePkg/Universal/EbcDxe/EbcDebugger/EdbSupportString.c
> @@ -323,21 +323,6 @@ UnicodeToUpper (
>    return (Chr >= L'a' && Chr <= L'z') ? Chr - (L'a' - L'A') : Chr;
>  }
> 
> -/**
> -
> -  Convert the character to upper case.
> -
> -  @param  Chr    the character to be converted.
> -
> -**/
> -STATIC
> -CHAR8
> -AsciiToUpper (
> -  IN      CHAR8                     Chr
> -  )
> -{
> -  return (Chr >= 'a' && Chr <= 'z') ? Chr - ('a' - 'A') : Chr;
> -}
> 
>  /**
>    Compare the Unicode and Ascii string pointed by String to the string
> pointed by String2.
> --
> 2.18.0.windows.1