[PATCH] MdeModulePkg/DumpDynPcd: Add a new application to dump dynamic PCD settings

[PATCH] MdeModulePkg/DumpDynPcd: Add a new application to dump dynamic PCD settings

[Zhang, Shenglei]

This is a shell application to dump dynamic PCD settings.
Type DumpDynPcd -?/h/H to get help information.
Type DumpDynPcd -v/V to get version information.
After test, this application can be run on NT32, OVMF and
KabyLake.
https://bugzilla.tianocore.org/show_bug.cgi?id=1541

Cc: Jian J Wang <jian.j.wang@intel.com>
Cc: Hao Wu <hao.a.wu@intel.com>
Cc: Ray Ni <ray.ni@intel.com>
Cc: Star Zeng <star.zeng@intel.com>
Signed-off-by: Shenglei Zhang <shenglei.zhang@intel.com>
---
 .../Application/DumpDynPcd/DumpDynPcd.c       | 608 ++++++++++++++++++
 .../Application/DumpDynPcd/DumpDynPcd.inf     |  51 ++
 .../Application/DumpDynPcd/DumpDynPcdStr.uni  |  28 +
 MdeModulePkg/MdeModulePkg.dsc                 |   1 +
 4 files changed, 688 insertions(+)
 create mode 100644 MdeModulePkg/Application/DumpDynPcd/DumpDynPcd.c
 create mode 100644 MdeModulePkg/Application/DumpDynPcd/DumpDynPcd.inf
 create mode 100644 MdeModulePkg/Application/DumpDynPcd/DumpDynPcdStr.uni

diff --git a/MdeModulePkg/Application/DumpDynPcd/DumpDynPcd.c b/MdeModulePkg/Application/DumpDynPcd/DumpDynPcd.c
new file mode 100644
index 0000000000..9753c49311
--- /dev/null
+++ b/MdeModulePkg/Application/DumpDynPcd/DumpDynPcd.c
@@ -0,0 +1,608 @@
+/** @file
+  A shell application to dump dynamic PCD settings.
+
+  Copyright (c) 2019, Intel Corporation. All rights reserved.<BR>
+  SPDX-License-Identifier: BSD-2-Clause-Patent
+
+**/
+
+#include <Uefi.h>
+#include <PiDxe.h>
+#include <Library/BaseLib.h>
+#include <Library/DebugLib.h>
+#include <Library/MemoryAllocationLib.h>
+#include <Library/UefiBootServicesTableLib.h>
+#include <Library/UefiLib.h>
+
+
+#include <Protocol/UnicodeCollation.h>
+#include <Protocol/PiPcd.h>
+#include <Protocol/Pcd.h>
+#include <Protocol/PiPcdInfo.h>
+#include <Protocol/PcdInfo.h>
+#include <Protocol/ShellParameters.h>
+#include <Protocol/Shell.h>
+
+
+//
+// String token ID of help message text.
+// Shell supports to find help message in the resource section of an application image if
+// .MAN file is not found. This global variable is added to make build tool recognizes
+// that the help string is consumed by user and then build tool will add the string into
+// the resource section. Thus the application can use '-?' option to show help message in
+// Shell.
+//
+GLOBAL_REMOVE_IF_UNREFERENCED EFI_STRING_ID mStrDumpDynPcdHelpTokenId = STRING_TOKEN (STR_DUMP_DYN_PCD_HELP_INFORMATION);
+
+#define MAJOR_VERSION   1
+#define MINOR_VERSION   0
+
+EFI_UNICODE_COLLATION_PROTOCOL  *mUnicodeCollation  = NULL;
+EFI_PCD_PROTOCOL                *mPiPcd             = NULL;
+PCD_PROTOCOL                    *mPcd               = NULL;
+EFI_GET_PCD_INFO_PROTOCOL       *mPiPcdInfo         = NULL;
+GET_PCD_INFO_PROTOCOL           *mPcdInfo           = NULL;
+CHAR16                          *mTempPcdNameBuffer = NULL;
+UINTN                           mTempPcdNameBufferSize = 0;
+CONST CHAR8 mHex[] = {'0', '1', '2', '3', '4', '5', '6', '7', '8', '9', 'A', 'B', 'C', 'D', 'E', 'F'};
+
+UINTN  Argc;
+CHAR16 **Argv;
+
+
+/**
+
+  This function parse application ARG.
+
+  @return Status
+**/
+EFI_STATUS
+GetArg (
+  VOID
+  )
+{
+  EFI_STATUS                    Status;
+  EFI_SHELL_PARAMETERS_PROTOCOL *ShellParameters;
+
+  Status = gBS->HandleProtocol (
+                  gImageHandle,
+                  &gEfiShellParametersProtocolGuid,
+                  (VOID**)&ShellParameters
+                  );
+  if (EFI_ERROR(Status)) {
+    return Status;
+  }
+
+  Argc = ShellParameters->Argc;
+  Argv = ShellParameters->Argv;
+  return EFI_SUCCESS;
+}
+
+/**
+   Display current version.
+**/
+VOID
+ShowVersion (
+  )
+{
+  Print (L"DumpDynPcd Version %d.%02d\n", MAJOR_VERSION, MINOR_VERSION);
+}
+
+/**
+   Display Usage and Help information.
+**/
+VOID
+ShowHelp (
+  )
+{
+  Print (L"Dump dynamic[ex] PCD info.\n");
+  Print (L"\n");
+  Print (L"DumpDynPcd [PcdName]\n");
+  Print (L"\n");
+  Print (L"  PcdName    Specifies the name of PCD.\n");
+  Print (L"             A literal[or partial] name or a pattern as specified in\n");
+  Print (L"             the MetaiMatch() function of the EFI_UNICODE_COLLATION2_PROCOOL.\n");
+  Print (L"             If it is absent, dump all PCDs' info.\n");
+  Print (L"The PCD data is printed as hexadecimal dump.\n");
+}
+
+/**
+  Dump some hexadecimal data to the screen.
+
+  @param[in] Indent     How many spaces to indent the output.
+  @param[in] Offset     The offset of the printing.
+  @param[in] DataSize   The size in bytes of UserData.
+  @param[in] UserData   The data to print out.
+**/
+VOID
+DumpHex (
+  IN UINTN        Indent,
+  IN UINTN        Offset,
+  IN UINTN        DataSize,
+  IN VOID         *UserData
+  )
+{
+  UINT8 *Data;
+
+  CHAR8 Val[50];
+
+  CHAR8 Str[20];
+
+  UINT8 TempByte;
+  UINTN Size;
+  UINTN Index;
+
+  Data = UserData;
+  while (DataSize != 0) {
+    Size = 16;
+    if (Size > DataSize) {
+      Size = DataSize;
+    }
+
+    for (Index = 0; Index < Size; Index += 1) {
+      TempByte            = Data[Index];
+      Val[Index * 3 + 0]  = mHex[TempByte >> 4];
+      Val[Index * 3 + 1]  = mHex[TempByte & 0xF];
+      Val[Index * 3 + 2]  = (CHAR8) ((Index == 7) ? '-' : ' ');
+      Str[Index]          = (CHAR8) ((TempByte < ' ' || TempByte > 'z') ? '.' : TempByte);
+    }
+
+    Val[Index * 3]  = 0;
+    Str[Index]      = 0;
+    Print (L"%*a%08X: %-48a *%a*\r\n", Indent, "", Offset, Val, Str);
+
+    Data += Size;
+    Offset += Size;
+    DataSize -= Size;
+  }
+}
+
+
+/**
+  Safely append with automatic string resizing given length of Destination and
+  desired length of copy from Source.
+
+  append the first D characters of Source to the end of Destination, where D is
+  the lesser of Count and the StrLen() of Source. If appending those D characters
+  will fit within Destination (whose Size is given as CurrentSize) and
+  still leave room for a NULL terminator, then those characters are appended,
+  starting at the original terminating NULL of Destination, and a new terminating
+  NULL is appended.
+
+  If appending D characters onto Destination will result in a overflow of the size
+  given in CurrentSize the string will be grown such that the copy can be performed
+  and CurrentSize will be updated to the new size.
+
+  If Source is NULL, there is nothing to append, just return the current buffer in
+  Destination.
+
+  if Destination is NULL, then ASSERT()
+  if Destination's current length (including NULL terminator) is already more then
+  CurrentSize, then ASSERT()
+
+  @param[in, out] Destination   The String to append onto
+  @param[in, out] CurrentSize   on call the number of bytes in Destination.  On
+                                return possibly the new size (still in bytes).  if NULL
+                                then allocate whatever is needed.
+  @param[in]      Source        The String to append from
+  @param[in]      Count         Maximum number of characters to append.  if 0 then
+                                all are appended.
+
+  @return Destination           return the resultant string.
+**/
+CHAR16*
+EFIAPI
+InternalStrnCatGrow (
+  IN OUT CHAR16           **Destination,
+  IN OUT UINTN            *CurrentSize,
+  IN     CONST CHAR16     *Source,
+  IN     UINTN            Count
+  )
+{
+  UINTN DestinationStartSize;
+  UINTN NewSize;
+
+  //
+  // ASSERTs
+  //
+  ASSERT(Destination != NULL);
+
+  //
+  // If there's nothing to do then just return Destination
+  //
+  if (Source == NULL) {
+    return (*Destination);
+  }
+
+  //
+  // allow for un-initialized pointers, based on size being 0
+  //
+  if (CurrentSize != NULL && *CurrentSize == 0) {
+    *Destination = NULL;
+  }
+
+  //
+  // allow for NULL pointers address as Destination
+  //
+  if (*Destination != NULL) {
+    ASSERT(CurrentSize != 0);
+    DestinationStartSize = StrSize(*Destination);
+    ASSERT(DestinationStartSize <= *CurrentSize);
+  } else {
+    DestinationStartSize = 0;
+//    ASSERT(*CurrentSize == 0);
+  }
+
+  //
+  // Append all of Source?
+  //
+  if (Count == 0) {
+    Count = StrLen(Source);
+  }
+
+  //
+  // Test and grow if required
+  //
+  if (CurrentSize != NULL) {
+    NewSize = *CurrentSize;
+    if (NewSize < DestinationStartSize + (Count * sizeof(CHAR16))) {
+      while (NewSize < (DestinationStartSize + (Count*sizeof(CHAR16)))) {
+        NewSize += 2 * Count * sizeof(CHAR16);
+      }
+      *Destination = ReallocatePool(*CurrentSize, NewSize, *Destination);
+      *CurrentSize = NewSize;
+    }
+  } else {
+    NewSize = (Count+1)*sizeof(CHAR16);
+    *Destination = AllocateZeroPool(NewSize);
+  }
+
+  //
+  // Now use standard StrnCat on a big enough buffer
+  //
+  if (*Destination == NULL) {
+    return (NULL);
+  }
+
+  StrnCatS(*Destination, NewSize/sizeof(CHAR16), Source, Count);
+  return *Destination;
+}
+
+/**
+  Get PCD type string based on input PCD type.
+
+  @param[in]    TokenSpace      PCD Token Space.
+  @param[in]    PcdType         The input PCD type.
+
+  @return       Pointer to PCD type string.
+**/
+CHAR16 *
+GetPcdTypeString (
+  IN CONST EFI_GUID     *TokenSpace,
+  IN EFI_PCD_TYPE       PcdType
+  )
+{
+  UINTN  BufLen;
+  CHAR16 *RetString;
+
+  BufLen      = 0;
+  RetString   = NULL;
+
+  switch (PcdType) {
+    case EFI_PCD_TYPE_8:
+      InternalStrnCatGrow (&RetString, &BufLen, L"UINT8", 0);
+      break;
+    case EFI_PCD_TYPE_16:
+      InternalStrnCatGrow (&RetString, &BufLen, L"UINT16", 0);
+      break;
+    case EFI_PCD_TYPE_32:
+      InternalStrnCatGrow (&RetString, &BufLen, L"UINT32", 0);
+      break;
+    case EFI_PCD_TYPE_64:
+      InternalStrnCatGrow (&RetString, &BufLen, L"UINT64", 0);
+      break;
+    case EFI_PCD_TYPE_BOOL:
+      InternalStrnCatGrow (&RetString, &BufLen, L"BOOLEAN", 0);
+      break;
+    case EFI_PCD_TYPE_PTR:
+      InternalStrnCatGrow (&RetString, &BufLen, L"POINTER", 0);
+      break;
+    default:
+      InternalStrnCatGrow (&RetString, &BufLen, L"UNKNOWN", 0);
+      break;
+  }
+
+  if (TokenSpace == NULL) {
+    InternalStrnCatGrow (&RetString, &BufLen, L":DYNAMIC", 0);
+  } else {
+    InternalStrnCatGrow (&RetString, &BufLen, L":DYNAMICEX", 0);
+  }
+
+  return RetString;
+}
+
+/**
+  Dump PCD info.
+
+  @param[in]    TokenSpace      PCD Token Space.
+  @param[in]    TokenNumber     PCD Token Number.
+  @param[in]    PcdInfo         Pointer to PCD info.
+**/
+VOID
+DumpPcdInfo (
+  IN CONST EFI_GUID     *TokenSpace,
+  IN UINTN              TokenNumber,
+  IN EFI_PCD_INFO       *PcdInfo
+  )
+{
+  CHAR16                *RetString;
+  UINT8                 Uint8;
+  UINT16                Uint16;
+  UINT32                Uint32;
+  UINT64                Uint64;
+  BOOLEAN               Boolean;
+  VOID                  *PcdData;
+
+  RetString = NULL;
+
+  if (PcdInfo->PcdName != NULL) {
+    Print (L"%a\n", PcdInfo->PcdName);
+  } else {
+    if (TokenSpace == NULL) {
+      Print (L"Default Token Space\n");
+    } else {
+      Print (L"%g\n", TokenSpace);
+    }
+  }
+
+  RetString = GetPcdTypeString (TokenSpace, PcdInfo->PcdType);
+
+  switch (PcdInfo->PcdType) {
+    case EFI_PCD_TYPE_8:
+      if (TokenSpace == NULL) {
+        Uint8 = mPcd->Get8 (TokenNumber);
+      } else {
+        Uint8 = mPiPcd->Get8 (TokenSpace, TokenNumber);
+      }
+      Print (L"  Token = 0x%08x - Type = %H%-17s%N - Size = 0x%x - Value = 0x%x\n", TokenNumber, RetString, PcdInfo->PcdSize, Uint8);
+      break;
+    case EFI_PCD_TYPE_16:
+      if (TokenSpace == NULL) {
+        Uint16 = mPcd->Get16 (TokenNumber);
+      } else {
+        Uint16 = mPiPcd->Get16 (TokenSpace, TokenNumber);
+      }
+      Print (L"  Token = 0x%08x - Type = %H%-17s%N - Size = 0x%x - Value = 0x%x\n", TokenNumber, RetString, PcdInfo->PcdSize, Uint16);
+      break;
+    case EFI_PCD_TYPE_32:
+      if (TokenSpace == NULL) {
+        Uint32 = mPcd->Get32 (TokenNumber);
+      } else {
+        Uint32 = mPiPcd->Get32 (TokenSpace, TokenNumber);
+      }
+      Print (L"  Token = 0x%08x - Type = %H%-17s%N - Size = 0x%x - Value = 0x%x\n", TokenNumber, RetString, PcdInfo->PcdSize, Uint32);
+      break;
+    case EFI_PCD_TYPE_64:
+      if (TokenSpace == NULL) {
+        Uint64 = mPcd->Get64 (TokenNumber);
+      } else {
+        Uint64 = mPiPcd->Get64 (TokenSpace, TokenNumber);
+      }
+      Print (L"  Token = 0x%08x - Type = %H%-17s%N - Size = 0x%x - Value = 0x%lx\n", TokenNumber, RetString, PcdInfo->PcdSize, Uint64);
+      break;
+    case EFI_PCD_TYPE_BOOL:
+      if (TokenSpace == NULL) {
+        Boolean = mPcd->GetBool (TokenNumber);
+      } else {
+        Boolean = mPiPcd->GetBool (TokenSpace, TokenNumber);
+      }
+      Print (L"  Token = 0x%08x - Type = %H%-17s%N - Size = 0x%x - Value = %a\n", TokenNumber, RetString, PcdInfo->PcdSize, Boolean ? "TRUE" : "FALSE");
+      break;
+    case EFI_PCD_TYPE_PTR:
+      if (TokenSpace == NULL) {
+        PcdData = mPcd->GetPtr (TokenNumber);
+      } else {
+        PcdData = mPiPcd->GetPtr (TokenSpace, TokenNumber);
+      }
+      Print (L"  Token = 0x%08x - Type = %H%-17s%N - Size = 0x%x\n", TokenNumber, RetString, PcdInfo->PcdSize);
+      DumpHex (2, 0, PcdInfo->PcdSize, PcdData);
+      break;
+    default:
+      return;
+  }
+
+  if (RetString != NULL) {
+    FreePool (RetString);
+  }
+  Print (L"\n");
+}
+
+/**
+  Show one or all PCDs' info.
+
+  @param[in]  InputPcdName       Pointer to PCD name to show. If NULL, show all PCDs' info.
+
+  @retval EFI_SUCCESS            Command completed successfully.
+  @retval EFI_OUT_OF_RESOURCES   Not enough resources were available to run the command.
+  @retval EFI_ABORTED            Aborted by user.
+  @retval EFI_NOT_FOUND          The specified PCD is not found.
+**/
+EFI_STATUS
+ProcessPcd (
+  IN CHAR16     *InputPcdName
+  )
+{
+  EFI_STATUS            Status;
+  EFI_GUID              *TokenSpace;
+  UINTN                 TokenNumber;
+  EFI_PCD_INFO          PcdInfo;
+  BOOLEAN               Found;
+  UINTN                 PcdNameSize;
+
+  PcdInfo.PcdName = NULL;
+  PcdInfo.PcdSize = 0;
+  PcdInfo.PcdType = 0xFF;
+  Found = FALSE;
+
+  Print (L"Current system SKU ID: 0x%x\n\n", mPiPcdInfo->GetSku ());
+
+  TokenSpace = NULL;
+  do {
+    TokenNumber = 0;
+    do {
+      Status = mPiPcd->GetNextToken (TokenSpace, &TokenNumber);
+      if (!EFI_ERROR (Status) && TokenNumber != 0) {
+        if (TokenSpace == NULL) {
+          //
+          // PCD in default Token Space.
+          //
+          mPcdInfo->GetInfo (TokenNumber, &PcdInfo);
+        } else {
+          mPiPcdInfo->GetInfo (TokenSpace, TokenNumber, &PcdInfo);
+        }
+        if (InputPcdName != NULL) {
+          if (PcdInfo.PcdName == NULL) {
+            continue;
+          }
+          PcdNameSize = AsciiStrSize (PcdInfo.PcdName) * sizeof (CHAR16);
+          if (mTempPcdNameBuffer == NULL) {
+            mTempPcdNameBufferSize = PcdNameSize;
+            mTempPcdNameBuffer = AllocatePool (mTempPcdNameBufferSize);
+          } else if (mTempPcdNameBufferSize < PcdNameSize) {
+            mTempPcdNameBuffer = ReallocatePool (mTempPcdNameBufferSize, PcdNameSize, mTempPcdNameBuffer);
+            mTempPcdNameBufferSize = PcdNameSize;
+          }
+          if (mTempPcdNameBuffer == NULL) {
+            return EFI_OUT_OF_RESOURCES;
+          }
+          AsciiStrToUnicodeStrS (PcdInfo.PcdName, mTempPcdNameBuffer, mTempPcdNameBufferSize / sizeof (CHAR16));
+          //
+          // Compare the input PCD name with the PCD name in PCD database.
+          //
+          if ((StrStr (mTempPcdNameBuffer, InputPcdName) != NULL) ||
+              (mUnicodeCollation != NULL && mUnicodeCollation->MetaiMatch (mUnicodeCollation, mTempPcdNameBuffer, InputPcdName))) {
+            //
+            // Found matched PCD.
+            //
+            DumpPcdInfo (TokenSpace, TokenNumber, &PcdInfo);
+            Found = TRUE;
+          }
+        } else {
+          DumpPcdInfo (TokenSpace, TokenNumber, &PcdInfo);
+        }
+      }
+    } while (!EFI_ERROR (Status) && TokenNumber != 0);
+
+    Status = mPiPcd->GetNextTokenSpace ((CONST EFI_GUID **) &TokenSpace);
+  } while (!EFI_ERROR (Status) && TokenSpace != NULL);
+
+  if ((InputPcdName != NULL) && !Found) {
+    //
+    // The specified PCD is not found, print error.
+    //
+    Print (L"%EError. %NNo matching PCD found: %s.\n", InputPcdName);
+    return EFI_NOT_FOUND;
+  }
+  return EFI_SUCCESS;
+}
+
+/**
+  Main entrypoint for DumpDynPcd shell application.
+
+  @param[in]  ImageHandle     The image handle.
+  @param[in]  SystemTable     The system table.
+
+  @retval EFI_SUCCESS            Command completed successfully.
+  @retval EFI_INVALID_PARAMETER  Command usage error.
+  @retval EFI_OUT_OF_RESOURCES   Not enough resources were available to run the command.
+  @retval EFI_ABORTED            Aborted by user.
+  @retval EFI_NOT_FOUND          The specified PCD is not found.
+  @retval Others                 Error status returned from gBS->LocateProtocol.
+**/
+EFI_STATUS
+EFIAPI
+DumpDynPcdMain (
+  IN EFI_HANDLE        ImageHandle,
+  IN EFI_SYSTEM_TABLE  *SystemTable
+  )
+{
+  EFI_STATUS    Status;
+  CHAR16        *InputPcdName;
+
+  InputPcdName  = NULL;
+
+  Status = gBS->LocateProtocol(&gEfiUnicodeCollation2ProtocolGuid, NULL, (VOID **) &mUnicodeCollation);
+  if (EFI_ERROR (Status)) {
+    mUnicodeCollation = NULL;
+  }
+
+  Status = gBS->LocateProtocol (&gEfiPcdProtocolGuid, NULL, (VOID **) &mPiPcd);
+  if (EFI_ERROR (Status)) {
+    Print (L"DumpDynPcd: %EError. %NPI PCD protocol is not present.\n");
+    return Status;
+  }
+
+  Status = gBS->LocateProtocol (&gEfiGetPcdInfoProtocolGuid, NULL, (VOID **) &mPiPcdInfo);
+  if (EFI_ERROR (Status)) {
+    Print (L"DumpDynPcd: %EError. %NPI PCD info protocol is not present.\n");
+    return Status;
+  }
+
+  Status = gBS->LocateProtocol (&gPcdProtocolGuid, NULL, (VOID **) &mPcd);
+  if (EFI_ERROR (Status)) {
+    Print (L"DumpDynPcd: %EError. %NPCD protocol is not present.\n");
+    return Status;
+  }
+
+  Status = gBS->LocateProtocol (&gGetPcdInfoProtocolGuid, NULL, (VOID **) &mPcdInfo);
+  if (EFI_ERROR (Status)) {
+    Print (L"DumpDynPcd: %EError. %NPCD info protocol is not present.\n");
+    return Status;
+  }
+
+  //
+  // get the command line arguments
+  //
+  Status = GetArg();
+  if (EFI_ERROR(Status)){
+    Print (L"DumpDynPcd: %EError. %NThe input parameters are not recognized.\n");
+    Status = EFI_INVALID_PARAMETER;
+    return Status;
+  }
+
+  if (Argc > 2){
+    Print (L"DumpDynPcd: %EError. %NToo many arguments specified.\n");
+    Status = EFI_INVALID_PARAMETER;
+    return Status;
+  }
+
+  if (Argc == 1){
+    Status = ProcessPcd (InputPcdName);
+    goto Done;
+  }
+
+  if ((StrCmp(Argv[1], L"-?") == 0)||(StrCmp(Argv[1], L"-h") == 0)||(StrCmp(Argv[1], L"-H") == 0)){
+    ShowHelp ();
+    goto Done;
+  } else
+    if ((StrCmp(Argv[1], L"-v") == 0)||(StrCmp(Argv[1], L"-V") == 0)){
+      ShowVersion ();
+      goto Done;
+    } else
+      if (StrStr(Argv[1], L"-") != NULL){
+        Print (L"DumpDynPcd: %EError. %NThe argument '%B%s%N' is invalid.\n", Argv[1]);
+        goto Done;
+      }
+
+  InputPcdName = Argv[1];
+  Status = ProcessPcd (InputPcdName);
+
+  Done:
+
+  if (mTempPcdNameBuffer != NULL) {
+    FreePool (mTempPcdNameBuffer);
+  }
+
+  return Status;
+}
+
diff --git a/MdeModulePkg/Application/DumpDynPcd/DumpDynPcd.inf b/MdeModulePkg/Application/DumpDynPcd/DumpDynPcd.inf
new file mode 100644
index 0000000000..af63eccca7
--- /dev/null
+++ b/MdeModulePkg/Application/DumpDynPcd/DumpDynPcd.inf
@@ -0,0 +1,51 @@
+#
+#  DumpDynPcd is a shell application to dump dynamic pcd information.
+#
+#  Copyright (c) 2019, Intel Corporation. All rights reserved.<BR>
+#  SPDX-License-Identifier: BSD-2-Clause-Patent
+#
+##
+
+[Defines]
+  INF_VERSION                    = 0x00010006
+  BASE_NAME                      = DumpDynPcd
+  FILE_GUID                      = 31ADA2B2-62EA-4866-9B87-03FEA8425974
+  MODULE_TYPE                    = UEFI_APPLICATION
+  VERSION_STRING                 = 1.0
+  ENTRY_POINT                    = DumpDynPcdMain
+
+#
+# This flag specifies whether HII resource section is generated into PE image.
+#
+  UEFI_HII_RESOURCE_SECTION      = TRUE
+
+#
+# The following information is for reference only and not required by the build tools.
+#
+#  VALID_ARCHITECTURES           = IA32 X64 IPF EBC
+#
+
+[Sources]
+  DumpDynPcd.c
+  DumpDynPcdStr.uni
+
+[Packages]
+  MdePkg/MdePkg.dec
+  ShellPkg/ShellPkg.dec
+
+[LibraryClasses]
+  BaseLib
+  UefiApplicationEntryPoint
+  DebugLib
+  MemoryAllocationLib
+  UefiLib
+  UefiBootServicesTableLib
+
+[Protocols]
+  gEfiUnicodeCollation2ProtocolGuid     ## SOMETIMES_CONSUMES
+  gEfiPcdProtocolGuid                   ## CONSUMES
+  gPcdProtocolGuid                      ## CONSUMES
+  gEfiGetPcdInfoProtocolGuid            ## CONSUMES
+  gGetPcdInfoProtocolGuid               ## CONSUMES
+  gEfiShellParametersProtocolGuid       ## CONSUMES
+
diff --git a/MdeModulePkg/Application/DumpDynPcd/DumpDynPcdStr.uni b/MdeModulePkg/Application/DumpDynPcd/DumpDynPcdStr.uni
new file mode 100644
index 0000000000..f0ee98219b
--- /dev/null
+++ b/MdeModulePkg/Application/DumpDynPcd/DumpDynPcdStr.uni
@@ -0,0 +1,28 @@
+//
+// DumpDynPcd is a shell application to dump dynamic pcd information.
+//
+// Copyright (c) 2019, Intel Corporation. All rights reserved.<BR>
+// SPDX-License-Identifier: BSD-2-Clause-Patent
+//
+//**/
+
+/=#
+
+#langdef en-US "English"
+
+#string STR_DUMP_DYN_PCD_HELP_INFORMATION       #language en-US ""
+                                                                ".TH DumpDynPcd 0 "Dump dynamic[ex] PCD info."\r\n"
+                                                                ".SH NAME\r\n"
+                                                                "Dump dynamic[ex] PCD info.\r\n"
+                                                                ".SH SYNOPSIS\r\n"
+                                                                " \r\n"
+                                                                "DumpDynPcd [PcdName].\r\n"
+                                                                ".SH OPTIONS\r\n"
+                                                                " \r\n"
+                                                                "  PcdName    Specifies the name of PCD.\r\n"
+                                                                "             A literal[or partial] name or a pattern as specified in\r\n"
+                                                                "             the MetaiMatch() function of the EFI_UNICODE_COLLATION2_PROCOOL.\r\n"
+                                                                "             If it is absent, dump all PCDs' info.\r\n"
+                                                                "The PCD data is printed as hexadecimal dump.\n"
+                                                                "\r\n"
+
diff --git a/MdeModulePkg/MdeModulePkg.dsc b/MdeModulePkg/MdeModulePkg.dsc
index b302f4a4f3..a8b9d8d027 100644
--- a/MdeModulePkg/MdeModulePkg.dsc
+++ b/MdeModulePkg/MdeModulePkg.dsc
@@ -206,6 +206,7 @@
 
 [Components]
   MdeModulePkg/Application/HelloWorld/HelloWorld.inf
+  MdeModulePkg/Application/DumpDynPcd/DumpDynPcd.inf
   MdeModulePkg/Application/MemoryProfileInfo/MemoryProfileInfo.inf
 
   MdeModulePkg/Bus/Pci/PciHostBridgeDxe/PciHostBridgeDxe.inf
-- 
2.18.0.windows.1

Re: [PATCH] MdeModulePkg/DumpDynPcd: Add a new application to dump dynamic PCD settings

[Wu, Hao A]

Loop in Eric.

Hello Shenglei,

One general level comment, could you help to follow the recommendation in
https://edk2.groups.io/g/devel/message/39655?p=,,,20,0,0,0::Created,,UefiDebugLibStdErr,20,2,0,31318888

to add 'static' for global variables/functions whose scope is limited
within a single file?


Also, some inline comments below.

> -----Original Message-----
> From: Zhang, Shenglei
> Sent: Sunday, May 05, 2019 1:50 PM
> To: devel@edk2.groups.io
> Cc: Wang, Jian J; Wu, Hao A; Ni, Ray; Zeng, Star
> Subject: [PATCH] MdeModulePkg/DumpDynPcd: Add a new application to dump
> dynamic PCD settings

Could you help to shorten the title to:

MdeModulePkg/DumpDynPcd: Add application to dump dynamic PCD settings

so as to address the issue reported by PatchCheck script?

> 
> This is a shell application to dump dynamic PCD settings.
> Type DumpDynPcd -?/h/H to get help information.
> Type DumpDynPcd -v/V to get version information.

I think you can add the (normal) usage "DumpDynPcd [PcdName]" of this
application here as well.

> After test, this application can be run on NT32, OVMF and
> KabyLake.
> https://bugzilla.tianocore.org/show_bug.cgi?id=1541

According to the description in the above BZ:

'''
DumpDynPcd [-b] [PcdName]
-b         Display one screen at a time.
'''

But I do not see there is a handle for the '-b' option.

Hello Eric,

As the BZ tracker submitter, are you okay with the missing of above option
support?

> 
> Cc: Jian J Wang <jian.j.wang@intel.com>
> Cc: Hao Wu <hao.a.wu@intel.com>
> Cc: Ray Ni <ray.ni@intel.com>
> Cc: Star Zeng <star.zeng@intel.com>
> Signed-off-by: Shenglei Zhang <shenglei.zhang@intel.com>
> ---
>  .../Application/DumpDynPcd/DumpDynPcd.c       | 608 ++++++++++++++++++
>  .../Application/DumpDynPcd/DumpDynPcd.inf     |  51 ++
>  .../Application/DumpDynPcd/DumpDynPcdStr.uni  |  28 +
>  MdeModulePkg/MdeModulePkg.dsc                 |   1 +
>  4 files changed, 688 insertions(+)
>  create mode 100644
> MdeModulePkg/Application/DumpDynPcd/DumpDynPcd.c
>  create mode 100644
> MdeModulePkg/Application/DumpDynPcd/DumpDynPcd.inf
>  create mode 100644
> MdeModulePkg/Application/DumpDynPcd/DumpDynPcdStr.uni
> 
> diff --git a/MdeModulePkg/Application/DumpDynPcd/DumpDynPcd.c
> b/MdeModulePkg/Application/DumpDynPcd/DumpDynPcd.c
> new file mode 100644
> index 0000000000..9753c49311
> --- /dev/null
> +++ b/MdeModulePkg/Application/DumpDynPcd/DumpDynPcd.c
> @@ -0,0 +1,608 @@
> +/** @file
> +  A shell application to dump dynamic PCD settings.
> +
> +  Copyright (c) 2019, Intel Corporation. All rights reserved.<BR>
> +  SPDX-License-Identifier: BSD-2-Clause-Patent
> +
> +**/
> +
> +#include <Uefi.h>
> +#include <PiDxe.h>
> +#include <Library/BaseLib.h>
> +#include <Library/DebugLib.h>
> +#include <Library/MemoryAllocationLib.h>
> +#include <Library/UefiBootServicesTableLib.h>
> +#include <Library/UefiLib.h>
> +
> +
> +#include <Protocol/UnicodeCollation.h>
> +#include <Protocol/PiPcd.h>
> +#include <Protocol/Pcd.h>
> +#include <Protocol/PiPcdInfo.h>
> +#include <Protocol/PcdInfo.h>
> +#include <Protocol/ShellParameters.h>
> +#include <Protocol/Shell.h>
> +
> +
> +//
> +// String token ID of help message text.
> +// Shell supports to find help message in the resource section of an application
> image if
> +// .MAN file is not found. This global variable is added to make build tool
> recognizes
> +// that the help string is consumed by user and then build tool will add the
> string into
> +// the resource section. Thus the application can use '-?' option to show help
> message in
> +// Shell.
> +//
> +GLOBAL_REMOVE_IF_UNREFERENCED EFI_STRING_ID
> mStrDumpDynPcdHelpTokenId = STRING_TOKEN
> (STR_DUMP_DYN_PCD_HELP_INFORMATION);
> +
> +#define MAJOR_VERSION   1
> +#define MINOR_VERSION   0
> +
> +EFI_UNICODE_COLLATION_PROTOCOL  *mUnicodeCollation  = NULL;
> +EFI_PCD_PROTOCOL                *mPiPcd             = NULL;
> +PCD_PROTOCOL                    *mPcd               = NULL;
> +EFI_GET_PCD_INFO_PROTOCOL       *mPiPcdInfo         = NULL;
> +GET_PCD_INFO_PROTOCOL           *mPcdInfo           = NULL;
> +CHAR16                          *mTempPcdNameBuffer = NULL;
> +UINTN                           mTempPcdNameBufferSize = 0;
> +CONST CHAR8 mHex[] = {'0', '1', '2', '3', '4', '5', '6', '7', '8', '9', 'A', 'B', 'C', 'D',
> 'E', 'F'};
> +
> +UINTN  Argc;
> +CHAR16 **Argv;
> +
> +
> +/**
> +
> +  This function parse application ARG.
> +
> +  @return Status
> +**/
> +EFI_STATUS
> +GetArg (
> +  VOID
> +  )
> +{
> +  EFI_STATUS                    Status;
> +  EFI_SHELL_PARAMETERS_PROTOCOL *ShellParameters;
> +
> +  Status = gBS->HandleProtocol (
> +                  gImageHandle,
> +                  &gEfiShellParametersProtocolGuid,
> +                  (VOID**)&ShellParameters
> +                  );
> +  if (EFI_ERROR(Status)) {
> +    return Status;
> +  }
> +
> +  Argc = ShellParameters->Argc;
> +  Argv = ShellParameters->Argv;
> +  return EFI_SUCCESS;
> +}
> +
> +/**
> +   Display current version.
> +**/
> +VOID
> +ShowVersion (
> +  )
> +{
> +  Print (L"DumpDynPcd Version %d.%02d\n", MAJOR_VERSION,
> MINOR_VERSION);
> +}
> +
> +/**
> +   Display Usage and Help information.
> +**/
> +VOID
> +ShowHelp (
> +  )
> +{
> +  Print (L"Dump dynamic[ex] PCD info.\n");
> +  Print (L"\n");
> +  Print (L"DumpDynPcd [PcdName]\n");
> +  Print (L"\n");
> +  Print (L"  PcdName    Specifies the name of PCD.\n");
> +  Print (L"             A literal[or partial] name or a pattern as specified in\n");
> +  Print (L"             the MetaiMatch() function of the
> EFI_UNICODE_COLLATION2_PROCOOL.\n");
> +  Print (L"             If it is absent, dump all PCDs' info.\n");
> +  Print (L"The PCD data is printed as hexadecimal dump.\n");
> +}
> +
> +/**
> +  Dump some hexadecimal data to the screen.
> +
> +  @param[in] Indent     How many spaces to indent the output.
> +  @param[in] Offset     The offset of the printing.
> +  @param[in] DataSize   The size in bytes of UserData.
> +  @param[in] UserData   The data to print out.
> +**/
> +VOID
> +DumpHex (
> +  IN UINTN        Indent,
> +  IN UINTN        Offset,
> +  IN UINTN        DataSize,
> +  IN VOID         *UserData
> +  )
> +{
> +  UINT8 *Data;
> +
> +  CHAR8 Val[50];
> +
> +  CHAR8 Str[20];
> +
> +  UINT8 TempByte;
> +  UINTN Size;
> +  UINTN Index;
> +
> +  Data = UserData;
> +  while (DataSize != 0) {
> +    Size = 16;
> +    if (Size > DataSize) {
> +      Size = DataSize;
> +    }
> +
> +    for (Index = 0; Index < Size; Index += 1) {
> +      TempByte            = Data[Index];
> +      Val[Index * 3 + 0]  = mHex[TempByte >> 4];
> +      Val[Index * 3 + 1]  = mHex[TempByte & 0xF];
> +      Val[Index * 3 + 2]  = (CHAR8) ((Index == 7) ? '-' : ' ');
> +      Str[Index]          = (CHAR8) ((TempByte < ' ' || TempByte > 'z') ? '.' :
> TempByte);
> +    }
> +
> +    Val[Index * 3]  = 0;
> +    Str[Index]      = 0;
> +    Print (L"%*a%08X: %-48a *%a*\r\n", Indent, "", Offset, Val, Str);
> +
> +    Data += Size;
> +    Offset += Size;
> +    DataSize -= Size;
> +  }
> +}
> +
> +
> +/**
> +  Safely append with automatic string resizing given length of Destination and
> +  desired length of copy from Source.
> +
> +  append the first D characters of Source to the end of Destination, where D is
> +  the lesser of Count and the StrLen() of Source. If appending those D
> characters
> +  will fit within Destination (whose Size is given as CurrentSize) and
> +  still leave room for a NULL terminator, then those characters are appended,
> +  starting at the original terminating NULL of Destination, and a new
> terminating
> +  NULL is appended.
> +
> +  If appending D characters onto Destination will result in a overflow of the
> size
> +  given in CurrentSize the string will be grown such that the copy can be
> performed
> +  and CurrentSize will be updated to the new size.
> +
> +  If Source is NULL, there is nothing to append, just return the current buffer in
> +  Destination.
> +
> +  if Destination is NULL, then ASSERT()
> +  if Destination's current length (including NULL terminator) is already more
> then
> +  CurrentSize, then ASSERT()
> +
> +  @param[in, out] Destination   The String to append onto
> +  @param[in, out] CurrentSize   on call the number of bytes in Destination.  On
> +                                return possibly the new size (still in bytes).  if NULL
> +                                then allocate whatever is needed.
> +  @param[in]      Source        The String to append from
> +  @param[in]      Count         Maximum number of characters to append.  if 0
> then
> +                                all are appended.
> +
> +  @return Destination           return the resultant string.
> +**/
> +CHAR16*
> +EFIAPI
> +InternalStrnCatGrow (

This one is an application internal function, 'EFIAPI' is not needed here.

> +  IN OUT CHAR16           **Destination,
> +  IN OUT UINTN            *CurrentSize,
> +  IN     CONST CHAR16     *Source,
> +  IN     UINTN            Count

Seems to me that the 'Count' parameter is always set to 0 among the calls
within this proposed application. Maybe it is better to just eliminate it.

> +  )
> +{
> +  UINTN DestinationStartSize;
> +  UINTN NewSize;
> +
> +  //
> +  // ASSERTs
> +  //
> +  ASSERT(Destination != NULL);
> +
> +  //
> +  // If there's nothing to do then just return Destination
> +  //
> +  if (Source == NULL) {
> +    return (*Destination);
> +  }
> +
> +  //
> +  // allow for un-initialized pointers, based on size being 0
> +  //
> +  if (CurrentSize != NULL && *CurrentSize == 0) {
> +    *Destination = NULL;
> +  }
> +
> +  //
> +  // allow for NULL pointers address as Destination
> +  //
> +  if (*Destination != NULL) {
> +    ASSERT(CurrentSize != 0);
> +    DestinationStartSize = StrSize(*Destination);
> +    ASSERT(DestinationStartSize <= *CurrentSize);
> +  } else {
> +    DestinationStartSize = 0;
> +//    ASSERT(*CurrentSize == 0);
> +  }
> +
> +  //
> +  // Append all of Source?
> +  //
> +  if (Count == 0) {
> +    Count = StrLen(Source);
> +  }
> +
> +  //
> +  // Test and grow if required
> +  //
> +  if (CurrentSize != NULL) {
> +    NewSize = *CurrentSize;
> +    if (NewSize < DestinationStartSize + (Count * sizeof(CHAR16))) {
> +      while (NewSize < (DestinationStartSize + (Count*sizeof(CHAR16)))) {
> +        NewSize += 2 * Count * sizeof(CHAR16);
> +      }
> +      *Destination = ReallocatePool(*CurrentSize, NewSize, *Destination);
> +      *CurrentSize = NewSize;
> +    }
> +  } else {
> +    NewSize = (Count+1)*sizeof(CHAR16);
> +    *Destination = AllocateZeroPool(NewSize);
> +  }
> +
> +  //
> +  // Now use standard StrnCat on a big enough buffer
> +  //
> +  if (*Destination == NULL) {
> +    return (NULL);
> +  }
> +
> +  StrnCatS(*Destination, NewSize/sizeof(CHAR16), Source, Count);
> +  return *Destination;
> +}
> +
> +/**
> +  Get PCD type string based on input PCD type.
> +
> +  @param[in]    TokenSpace      PCD Token Space.
> +  @param[in]    PcdType         The input PCD type.
> +
> +  @return       Pointer to PCD type string.
> +**/
> +CHAR16 *
> +GetPcdTypeString (
> +  IN CONST EFI_GUID     *TokenSpace,
> +  IN EFI_PCD_TYPE       PcdType
> +  )
> +{
> +  UINTN  BufLen;
> +  CHAR16 *RetString;
> +
> +  BufLen      = 0;
> +  RetString   = NULL;
> +
> +  switch (PcdType) {
> +    case EFI_PCD_TYPE_8:
> +      InternalStrnCatGrow (&RetString, &BufLen, L"UINT8", 0);
> +      break;
> +    case EFI_PCD_TYPE_16:
> +      InternalStrnCatGrow (&RetString, &BufLen, L"UINT16", 0);
> +      break;
> +    case EFI_PCD_TYPE_32:
> +      InternalStrnCatGrow (&RetString, &BufLen, L"UINT32", 0);
> +      break;
> +    case EFI_PCD_TYPE_64:
> +      InternalStrnCatGrow (&RetString, &BufLen, L"UINT64", 0);
> +      break;
> +    case EFI_PCD_TYPE_BOOL:
> +      InternalStrnCatGrow (&RetString, &BufLen, L"BOOLEAN", 0);
> +      break;
> +    case EFI_PCD_TYPE_PTR:
> +      InternalStrnCatGrow (&RetString, &BufLen, L"POINTER", 0);
> +      break;
> +    default:
> +      InternalStrnCatGrow (&RetString, &BufLen, L"UNKNOWN", 0);
> +      break;
> +  }
> +
> +  if (TokenSpace == NULL) {
> +    InternalStrnCatGrow (&RetString, &BufLen, L":DYNAMIC", 0);
> +  } else {
> +    InternalStrnCatGrow (&RetString, &BufLen, L":DYNAMICEX", 0);
> +  }
> +
> +  return RetString;
> +}
> +
> +/**
> +  Dump PCD info.
> +
> +  @param[in]    TokenSpace      PCD Token Space.
> +  @param[in]    TokenNumber     PCD Token Number.
> +  @param[in]    PcdInfo         Pointer to PCD info.
> +**/
> +VOID
> +DumpPcdInfo (
> +  IN CONST EFI_GUID     *TokenSpace,
> +  IN UINTN              TokenNumber,
> +  IN EFI_PCD_INFO       *PcdInfo
> +  )
> +{
> +  CHAR16                *RetString;
> +  UINT8                 Uint8;
> +  UINT16                Uint16;
> +  UINT32                Uint32;
> +  UINT64                Uint64;
> +  BOOLEAN               Boolean;
> +  VOID                  *PcdData;
> +
> +  RetString = NULL;
> +
> +  if (PcdInfo->PcdName != NULL) {
> +    Print (L"%a\n", PcdInfo->PcdName);
> +  } else {
> +    if (TokenSpace == NULL) {
> +      Print (L"Default Token Space\n");
> +    } else {
> +      Print (L"%g\n", TokenSpace);
> +    }
> +  }
> +
> +  RetString = GetPcdTypeString (TokenSpace, PcdInfo->PcdType);
> +
> +  switch (PcdInfo->PcdType) {
> +    case EFI_PCD_TYPE_8:
> +      if (TokenSpace == NULL) {
> +        Uint8 = mPcd->Get8 (TokenNumber);
> +      } else {
> +        Uint8 = mPiPcd->Get8 (TokenSpace, TokenNumber);
> +      }
> +      Print (L"  Token = 0x%08x - Type = %H%-17s%N - Size = 0x%x - Value =
> 0x%x\n", TokenNumber, RetString, PcdInfo->PcdSize, Uint8);
> +      break;
> +    case EFI_PCD_TYPE_16:
> +      if (TokenSpace == NULL) {
> +        Uint16 = mPcd->Get16 (TokenNumber);
> +      } else {
> +        Uint16 = mPiPcd->Get16 (TokenSpace, TokenNumber);
> +      }
> +      Print (L"  Token = 0x%08x - Type = %H%-17s%N - Size = 0x%x - Value =
> 0x%x\n", TokenNumber, RetString, PcdInfo->PcdSize, Uint16);
> +      break;
> +    case EFI_PCD_TYPE_32:
> +      if (TokenSpace == NULL) {
> +        Uint32 = mPcd->Get32 (TokenNumber);
> +      } else {
> +        Uint32 = mPiPcd->Get32 (TokenSpace, TokenNumber);
> +      }
> +      Print (L"  Token = 0x%08x - Type = %H%-17s%N - Size = 0x%x - Value =
> 0x%x\n", TokenNumber, RetString, PcdInfo->PcdSize, Uint32);
> +      break;
> +    case EFI_PCD_TYPE_64:
> +      if (TokenSpace == NULL) {
> +        Uint64 = mPcd->Get64 (TokenNumber);
> +      } else {
> +        Uint64 = mPiPcd->Get64 (TokenSpace, TokenNumber);
> +      }
> +      Print (L"  Token = 0x%08x - Type = %H%-17s%N - Size = 0x%x - Value =
> 0x%lx\n", TokenNumber, RetString, PcdInfo->PcdSize, Uint64);
> +      break;
> +    case EFI_PCD_TYPE_BOOL:
> +      if (TokenSpace == NULL) {
> +        Boolean = mPcd->GetBool (TokenNumber);
> +      } else {
> +        Boolean = mPiPcd->GetBool (TokenSpace, TokenNumber);
> +      }
> +      Print (L"  Token = 0x%08x - Type = %H%-17s%N - Size = 0x%x - Value
> = %a\n", TokenNumber, RetString, PcdInfo->PcdSize, Boolean ? "TRUE" :
> "FALSE");
> +      break;
> +    case EFI_PCD_TYPE_PTR:
> +      if (TokenSpace == NULL) {
> +        PcdData = mPcd->GetPtr (TokenNumber);
> +      } else {
> +        PcdData = mPiPcd->GetPtr (TokenSpace, TokenNumber);
> +      }
> +      Print (L"  Token = 0x%08x - Type = %H%-17s%N - Size = 0x%x\n",
> TokenNumber, RetString, PcdInfo->PcdSize);
> +      DumpHex (2, 0, PcdInfo->PcdSize, PcdData);
> +      break;
> +    default:
> +      return;
> +  }
> +
> +  if (RetString != NULL) {
> +    FreePool (RetString);
> +  }
> +  Print (L"\n");
> +}
> +
> +/**
> +  Show one or all PCDs' info.
> +
> +  @param[in]  InputPcdName       Pointer to PCD name to show. If NULL, show
> all PCDs' info.
> +
> +  @retval EFI_SUCCESS            Command completed successfully.
> +  @retval EFI_OUT_OF_RESOURCES   Not enough resources were available to
> run the command.
> +  @retval EFI_ABORTED            Aborted by user.
> +  @retval EFI_NOT_FOUND          The specified PCD is not found.
> +**/
> +EFI_STATUS
> +ProcessPcd (
> +  IN CHAR16     *InputPcdName
> +  )
> +{
> +  EFI_STATUS            Status;
> +  EFI_GUID              *TokenSpace;
> +  UINTN                 TokenNumber;
> +  EFI_PCD_INFO          PcdInfo;
> +  BOOLEAN               Found;
> +  UINTN                 PcdNameSize;
> +
> +  PcdInfo.PcdName = NULL;
> +  PcdInfo.PcdSize = 0;
> +  PcdInfo.PcdType = 0xFF;
> +  Found = FALSE;
> +
> +  Print (L"Current system SKU ID: 0x%x\n\n", mPiPcdInfo->GetSku ());
> +
> +  TokenSpace = NULL;
> +  do {
> +    TokenNumber = 0;
> +    do {
> +      Status = mPiPcd->GetNextToken (TokenSpace, &TokenNumber);
> +      if (!EFI_ERROR (Status) && TokenNumber != 0) {
> +        if (TokenSpace == NULL) {
> +          //
> +          // PCD in default Token Space.
> +          //
> +          mPcdInfo->GetInfo (TokenNumber, &PcdInfo);
> +        } else {
> +          mPiPcdInfo->GetInfo (TokenSpace, TokenNumber, &PcdInfo);
> +        }
> +        if (InputPcdName != NULL) {
> +          if (PcdInfo.PcdName == NULL) {
> +            continue;
> +          }
> +          PcdNameSize = AsciiStrSize (PcdInfo.PcdName) * sizeof (CHAR16);
> +          if (mTempPcdNameBuffer == NULL) {
> +            mTempPcdNameBufferSize = PcdNameSize;
> +            mTempPcdNameBuffer = AllocatePool (mTempPcdNameBufferSize);
> +          } else if (mTempPcdNameBufferSize < PcdNameSize) {
> +            mTempPcdNameBuffer = ReallocatePool (mTempPcdNameBufferSize,
> PcdNameSize, mTempPcdNameBuffer);
> +            mTempPcdNameBufferSize = PcdNameSize;
> +          }
> +          if (mTempPcdNameBuffer == NULL) {
> +            return EFI_OUT_OF_RESOURCES;
> +          }
> +          AsciiStrToUnicodeStrS (PcdInfo.PcdName, mTempPcdNameBuffer,
> mTempPcdNameBufferSize / sizeof (CHAR16));
> +          //
> +          // Compare the input PCD name with the PCD name in PCD database.
> +          //
> +          if ((StrStr (mTempPcdNameBuffer, InputPcdName) != NULL) ||
> +              (mUnicodeCollation != NULL && mUnicodeCollation->MetaiMatch
> (mUnicodeCollation, mTempPcdNameBuffer, InputPcdName))) {
> +            //
> +            // Found matched PCD.
> +            //
> +            DumpPcdInfo (TokenSpace, TokenNumber, &PcdInfo);
> +            Found = TRUE;
> +          }
> +        } else {
> +          DumpPcdInfo (TokenSpace, TokenNumber, &PcdInfo);
> +        }
> +      }
> +    } while (!EFI_ERROR (Status) && TokenNumber != 0);
> +
> +    Status = mPiPcd->GetNextTokenSpace ((CONST EFI_GUID **) &TokenSpace);
> +  } while (!EFI_ERROR (Status) && TokenSpace != NULL);
> +
> +  if ((InputPcdName != NULL) && !Found) {
> +    //
> +    // The specified PCD is not found, print error.
> +    //
> +    Print (L"%EError. %NNo matching PCD found: %s.\n", InputPcdName);
> +    return EFI_NOT_FOUND;
> +  }
> +  return EFI_SUCCESS;
> +}
> +
> +/**
> +  Main entrypoint for DumpDynPcd shell application.
> +
> +  @param[in]  ImageHandle     The image handle.
> +  @param[in]  SystemTable     The system table.
> +
> +  @retval EFI_SUCCESS            Command completed successfully.
> +  @retval EFI_INVALID_PARAMETER  Command usage error.
> +  @retval EFI_OUT_OF_RESOURCES   Not enough resources were available to
> run the command.
> +  @retval EFI_ABORTED            Aborted by user.
> +  @retval EFI_NOT_FOUND          The specified PCD is not found.
> +  @retval Others                 Error status returned from gBS->LocateProtocol.
> +**/
> +EFI_STATUS
> +EFIAPI
> +DumpDynPcdMain (
> +  IN EFI_HANDLE        ImageHandle,
> +  IN EFI_SYSTEM_TABLE  *SystemTable
> +  )
> +{
> +  EFI_STATUS    Status;
> +  CHAR16        *InputPcdName;
> +
> +  InputPcdName  = NULL;
> +
> +  Status = gBS->LocateProtocol(&gEfiUnicodeCollation2ProtocolGuid, NULL,
> (VOID **) &mUnicodeCollation);
> +  if (EFI_ERROR (Status)) {
> +    mUnicodeCollation = NULL;
> +  }
> +
> +  Status = gBS->LocateProtocol (&gEfiPcdProtocolGuid, NULL, (VOID **)
> &mPiPcd);
> +  if (EFI_ERROR (Status)) {
> +    Print (L"DumpDynPcd: %EError. %NPI PCD protocol is not present.\n");
> +    return Status;
> +  }
> +
> +  Status = gBS->LocateProtocol (&gEfiGetPcdInfoProtocolGuid, NULL, (VOID **)
> &mPiPcdInfo);
> +  if (EFI_ERROR (Status)) {
> +    Print (L"DumpDynPcd: %EError. %NPI PCD info protocol is not present.\n");
> +    return Status;
> +  }
> +
> +  Status = gBS->LocateProtocol (&gPcdProtocolGuid, NULL, (VOID **) &mPcd);
> +  if (EFI_ERROR (Status)) {
> +    Print (L"DumpDynPcd: %EError. %NPCD protocol is not present.\n");
> +    return Status;
> +  }
> +
> +  Status = gBS->LocateProtocol (&gGetPcdInfoProtocolGuid, NULL, (VOID **)
> &mPcdInfo);
> +  if (EFI_ERROR (Status)) {
> +    Print (L"DumpDynPcd: %EError. %NPCD info protocol is not present.\n");
> +    return Status;
> +  }
> +
> +  //
> +  // get the command line arguments
> +  //
> +  Status = GetArg();
> +  if (EFI_ERROR(Status)){
> +    Print (L"DumpDynPcd: %EError. %NThe input parameters are not
> recognized.\n");
> +    Status = EFI_INVALID_PARAMETER;
> +    return Status;
> +  }
> +
> +  if (Argc > 2){
> +    Print (L"DumpDynPcd: %EError. %NToo many arguments specified.\n");
> +    Status = EFI_INVALID_PARAMETER;
> +    return Status;
> +  }
> +
> +  if (Argc == 1){
> +    Status = ProcessPcd (InputPcdName);
> +    goto Done;
> +  }
> +
> +  if ((StrCmp(Argv[1], L"-?") == 0)||(StrCmp(Argv[1], L"-h") ==
> 0)||(StrCmp(Argv[1], L"-H") == 0)){
> +    ShowHelp ();
> +    goto Done;
> +  } else

Please put an open brace after 'else'.

Seems to me there is a hard requirement according to EDK II C Coding
Standards Specification Version 2.1, Section 5.7.3.4.2:

'''
An open brace always follows the else
'''

> +    if ((StrCmp(Argv[1], L"-v") == 0)||(StrCmp(Argv[1], L"-V") == 0)){
> +      ShowVersion ();
> +      goto Done;
> +    } else

Similar comment as above.

> +      if (StrStr(Argv[1], L"-") != NULL){
> +        Print (L"DumpDynPcd: %EError. %NThe argument '%B%s%N' is invalid.\n",
> Argv[1]);
> +        goto Done;
> +      }
> +
> +  InputPcdName = Argv[1];
> +  Status = ProcessPcd (InputPcdName);
> +
> +  Done:
> +
> +  if (mTempPcdNameBuffer != NULL) {
> +    FreePool (mTempPcdNameBuffer);
> +  }
> +
> +  return Status;
> +}
> +
> diff --git a/MdeModulePkg/Application/DumpDynPcd/DumpDynPcd.inf
> b/MdeModulePkg/Application/DumpDynPcd/DumpDynPcd.inf
> new file mode 100644
> index 0000000000..af63eccca7
> --- /dev/null
> +++ b/MdeModulePkg/Application/DumpDynPcd/DumpDynPcd.inf
> @@ -0,0 +1,51 @@
> +#
> +#  DumpDynPcd is a shell application to dump dynamic pcd information.
> +#
> +#  Copyright (c) 2019, Intel Corporation. All rights reserved.<BR>
> +#  SPDX-License-Identifier: BSD-2-Clause-Patent
> +#
> +##
> +
> +[Defines]
> +  INF_VERSION                    = 0x00010006
> +  BASE_NAME                      = DumpDynPcd
> +  FILE_GUID                      = 31ADA2B2-62EA-4866-9B87-03FEA8425974
> +  MODULE_TYPE                    = UEFI_APPLICATION
> +  VERSION_STRING                 = 1.0
> +  ENTRY_POINT                    = DumpDynPcdMain
> +
> +#
> +# This flag specifies whether HII resource section is generated into PE image.
> +#
> +  UEFI_HII_RESOURCE_SECTION      = TRUE
> +
> +#
> +# The following information is for reference only and not required by the build
> tools.
> +#
> +#  VALID_ARCHITECTURES           = IA32 X64 IPF EBC

Please help to remove 'IPF' in the above comment.

> +#
> +
> +[Sources]
> +  DumpDynPcd.c
> +  DumpDynPcdStr.uni
> +
> +[Packages]
> +  MdePkg/MdePkg.dec
> +  ShellPkg/ShellPkg.dec

Please help to drop the ShellPkg.dec here.
I do not think there is any library/protocol dependency in ShellPkg.

Best Regards,
Hao Wu

> +
> +[LibraryClasses]
> +  BaseLib
> +  UefiApplicationEntryPoint
> +  DebugLib
> +  MemoryAllocationLib
> +  UefiLib
> +  UefiBootServicesTableLib
> +
> +[Protocols]
> +  gEfiUnicodeCollation2ProtocolGuid     ## SOMETIMES_CONSUMES
> +  gEfiPcdProtocolGuid                   ## CONSUMES
> +  gPcdProtocolGuid                      ## CONSUMES
> +  gEfiGetPcdInfoProtocolGuid            ## CONSUMES
> +  gGetPcdInfoProtocolGuid               ## CONSUMES
> +  gEfiShellParametersProtocolGuid       ## CONSUMES
> +
> diff --git a/MdeModulePkg/Application/DumpDynPcd/DumpDynPcdStr.uni
> b/MdeModulePkg/Application/DumpDynPcd/DumpDynPcdStr.uni
> new file mode 100644
> index 0000000000..f0ee98219b
> --- /dev/null
> +++ b/MdeModulePkg/Application/DumpDynPcd/DumpDynPcdStr.uni
> @@ -0,0 +1,28 @@
> +//
> +// DumpDynPcd is a shell application to dump dynamic pcd information.
> +//
> +// Copyright (c) 2019, Intel Corporation. All rights reserved.<BR>
> +// SPDX-License-Identifier: BSD-2-Clause-Patent
> +//
> +//**/
> +
> +/=#
> +
> +#langdef en-US "English"
> +
> +#string STR_DUMP_DYN_PCD_HELP_INFORMATION       #language en-US ""
> +                                                                ".TH DumpDynPcd 0 "Dump
> dynamic[ex] PCD info."\r\n"
> +                                                                ".SH NAME\r\n"
> +                                                                "Dump dynamic[ex] PCD info.\r\n"
> +                                                                ".SH SYNOPSIS\r\n"
> +                                                                " \r\n"
> +                                                                "DumpDynPcd [PcdName].\r\n"
> +                                                                ".SH OPTIONS\r\n"
> +                                                                " \r\n"
> +                                                                "  PcdName    Specifies the name of
> PCD.\r\n"
> +                                                                "             A literal[or partial] name or a
> pattern as specified in\r\n"
> +                                                                "             the MetaiMatch() function of
> the EFI_UNICODE_COLLATION2_PROCOOL.\r\n"
> +                                                                "             If it is absent, dump all PCDs'
> info.\r\n"
> +                                                                "The PCD data is printed as
> hexadecimal dump.\n"
> +                                                                "\r\n"
> +
> diff --git a/MdeModulePkg/MdeModulePkg.dsc
> b/MdeModulePkg/MdeModulePkg.dsc
> index b302f4a4f3..a8b9d8d027 100644
> --- a/MdeModulePkg/MdeModulePkg.dsc
> +++ b/MdeModulePkg/MdeModulePkg.dsc
> @@ -206,6 +206,7 @@
> 
>  [Components]
>    MdeModulePkg/Application/HelloWorld/HelloWorld.inf
> +  MdeModulePkg/Application/DumpDynPcd/DumpDynPcd.inf
>    MdeModulePkg/Application/MemoryProfileInfo/MemoryProfileInfo.inf
> 
>    MdeModulePkg/Bus/Pci/PciHostBridgeDxe/PciHostBridgeDxe.inf
> --
> 2.18.0.windows.1

Re: [PATCH] MdeModulePkg/DumpDynPcd: Add a new application to dump dynamic PCD settings

[Dong, Eric]

Hi Hao,

Shenglei has sync with me before about this gap. Both me and Liming is fine with this change.

Thanks,
Eric

> -----Original Message-----
> From: Wu, Hao A
> Sent: Monday, May 6, 2019 9:14 PM
> To: Zhang, Shenglei <shenglei.zhang@intel.com>; Dong, Eric
> <eric.dong@intel.com>; devel@edk2.groups.io
> Cc: Wang, Jian J <jian.j.wang@intel.com>; Ni, Ray <ray.ni@intel.com>; Zeng,
> Star <star.zeng@intel.com>
> Subject: RE: [PATCH] MdeModulePkg/DumpDynPcd: Add a new application
> to dump dynamic PCD settings
> 
> Loop in Eric.
> 
> Hello Shenglei,
> 
> One general level comment, could you help to follow the recommendation in
> https://edk2.groups.io/g/devel/message/39655?p=,,,20,0,0,0::Created,,Uefi
> DebugLibStdErr,20,2,0,31318888
> 
> to add 'static' for global variables/functions whose scope is limited within a
> single file?
> 
> 
> Also, some inline comments below.
> 
> > -----Original Message-----
> > From: Zhang, Shenglei
> > Sent: Sunday, May 05, 2019 1:50 PM
> > To: devel@edk2.groups.io
> > Cc: Wang, Jian J; Wu, Hao A; Ni, Ray; Zeng, Star
> > Subject: [PATCH] MdeModulePkg/DumpDynPcd: Add a new application to
> > dump dynamic PCD settings
> 
> Could you help to shorten the title to:
> 
> MdeModulePkg/DumpDynPcd: Add application to dump dynamic PCD
> settings
> 
> so as to address the issue reported by PatchCheck script?
> 
> >
> > This is a shell application to dump dynamic PCD settings.
> > Type DumpDynPcd -?/h/H to get help information.
> > Type DumpDynPcd -v/V to get version information.
> 
> I think you can add the (normal) usage "DumpDynPcd [PcdName]" of this
> application here as well.
> 
> > After test, this application can be run on NT32, OVMF and KabyLake.
> > https://bugzilla.tianocore.org/show_bug.cgi?id=1541
> 
> According to the description in the above BZ:
> 
> '''
> DumpDynPcd [-b] [PcdName]
> -b         Display one screen at a time.
> '''
> 
> But I do not see there is a handle for the '-b' option.
> 
> Hello Eric,
> 
> As the BZ tracker submitter, are you okay with the missing of above option
> support?
> 
> >
> > Cc: Jian J Wang <jian.j.wang@intel.com>
> > Cc: Hao Wu <hao.a.wu@intel.com>
> > Cc: Ray Ni <ray.ni@intel.com>
> > Cc: Star Zeng <star.zeng@intel.com>
> > Signed-off-by: Shenglei Zhang <shenglei.zhang@intel.com>
> > ---
> >  .../Application/DumpDynPcd/DumpDynPcd.c       | 608
> ++++++++++++++++++
> >  .../Application/DumpDynPcd/DumpDynPcd.inf     |  51 ++
> >  .../Application/DumpDynPcd/DumpDynPcdStr.uni  |  28 +
> >  MdeModulePkg/MdeModulePkg.dsc                 |   1 +
> >  4 files changed, 688 insertions(+)
> >  create mode 100644
> > MdeModulePkg/Application/DumpDynPcd/DumpDynPcd.c
> >  create mode 100644
> > MdeModulePkg/Application/DumpDynPcd/DumpDynPcd.inf
> >  create mode 100644
> > MdeModulePkg/Application/DumpDynPcd/DumpDynPcdStr.uni
> >
> > diff --git a/MdeModulePkg/Application/DumpDynPcd/DumpDynPcd.c
> > b/MdeModulePkg/Application/DumpDynPcd/DumpDynPcd.c
> > new file mode 100644
> > index 0000000000..9753c49311
> > --- /dev/null
> > +++ b/MdeModulePkg/Application/DumpDynPcd/DumpDynPcd.c
> > @@ -0,0 +1,608 @@
> > +/** @file
> > +  A shell application to dump dynamic PCD settings.
> > +
> > +  Copyright (c) 2019, Intel Corporation. All rights reserved.<BR>
> > +  SPDX-License-Identifier: BSD-2-Clause-Patent
> > +
> > +**/
> > +
> > +#include <Uefi.h>
> > +#include <PiDxe.h>
> > +#include <Library/BaseLib.h>
> > +#include <Library/DebugLib.h>
> > +#include <Library/MemoryAllocationLib.h> #include
> > +<Library/UefiBootServicesTableLib.h>
> > +#include <Library/UefiLib.h>
> > +
> > +
> > +#include <Protocol/UnicodeCollation.h> #include <Protocol/PiPcd.h>
> > +#include <Protocol/Pcd.h> #include <Protocol/PiPcdInfo.h> #include
> > +<Protocol/PcdInfo.h> #include <Protocol/ShellParameters.h> #include
> > +<Protocol/Shell.h>
> > +
> > +
> > +//
> > +// String token ID of help message text.
> > +// Shell supports to find help message in the resource section of an
> > +application
> > image if
> > +// .MAN file is not found. This global variable is added to make
> > +build tool
> > recognizes
> > +// that the help string is consumed by user and then build tool will
> > +add the
> > string into
> > +// the resource section. Thus the application can use '-?' option to
> > +show help
> > message in
> > +// Shell.
> > +//
> > +GLOBAL_REMOVE_IF_UNREFERENCED EFI_STRING_ID
> > mStrDumpDynPcdHelpTokenId = STRING_TOKEN
> > (STR_DUMP_DYN_PCD_HELP_INFORMATION);
> > +
> > +#define MAJOR_VERSION   1
> > +#define MINOR_VERSION   0
> > +
> > +EFI_UNICODE_COLLATION_PROTOCOL  *mUnicodeCollation  = NULL;
> > +EFI_PCD_PROTOCOL                *mPiPcd             = NULL;
> > +PCD_PROTOCOL                    *mPcd               = NULL;
> > +EFI_GET_PCD_INFO_PROTOCOL       *mPiPcdInfo         = NULL;
> > +GET_PCD_INFO_PROTOCOL           *mPcdInfo           = NULL;
> > +CHAR16                          *mTempPcdNameBuffer = NULL;
> > +UINTN                           mTempPcdNameBufferSize = 0;
> > +CONST CHAR8 mHex[] = {'0', '1', '2', '3', '4', '5', '6', '7', '8',
> > +'9', 'A', 'B', 'C', 'D',
> > 'E', 'F'};
> > +
> > +UINTN  Argc;
> > +CHAR16 **Argv;
> > +
> > +
> > +/**
> > +
> > +  This function parse application ARG.
> > +
> > +  @return Status
> > +**/
> > +EFI_STATUS
> > +GetArg (
> > +  VOID
> > +  )
> > +{
> > +  EFI_STATUS                    Status;
> > +  EFI_SHELL_PARAMETERS_PROTOCOL *ShellParameters;
> > +
> > +  Status = gBS->HandleProtocol (
> > +                  gImageHandle,
> > +                  &gEfiShellParametersProtocolGuid,
> > +                  (VOID**)&ShellParameters
> > +                  );
> > +  if (EFI_ERROR(Status)) {
> > +    return Status;
> > +  }
> > +
> > +  Argc = ShellParameters->Argc;
> > +  Argv = ShellParameters->Argv;
> > +  return EFI_SUCCESS;
> > +}
> > +
> > +/**
> > +   Display current version.
> > +**/
> > +VOID
> > +ShowVersion (
> > +  )
> > +{
> > +  Print (L"DumpDynPcd Version %d.%02d\n", MAJOR_VERSION,
> > MINOR_VERSION);
> > +}
> > +
> > +/**
> > +   Display Usage and Help information.
> > +**/
> > +VOID
> > +ShowHelp (
> > +  )
> > +{
> > +  Print (L"Dump dynamic[ex] PCD info.\n");
> > +  Print (L"\n");
> > +  Print (L"DumpDynPcd [PcdName]\n");
> > +  Print (L"\n");
> > +  Print (L"  PcdName    Specifies the name of PCD.\n");
> > +  Print (L"             A literal[or partial] name or a pattern as specified in\n");
> > +  Print (L"             the MetaiMatch() function of the
> > EFI_UNICODE_COLLATION2_PROCOOL.\n");
> > +  Print (L"             If it is absent, dump all PCDs' info.\n");
> > +  Print (L"The PCD data is printed as hexadecimal dump.\n"); }
> > +
> > +/**
> > +  Dump some hexadecimal data to the screen.
> > +
> > +  @param[in] Indent     How many spaces to indent the output.
> > +  @param[in] Offset     The offset of the printing.
> > +  @param[in] DataSize   The size in bytes of UserData.
> > +  @param[in] UserData   The data to print out.
> > +**/
> > +VOID
> > +DumpHex (
> > +  IN UINTN        Indent,
> > +  IN UINTN        Offset,
> > +  IN UINTN        DataSize,
> > +  IN VOID         *UserData
> > +  )
> > +{
> > +  UINT8 *Data;
> > +
> > +  CHAR8 Val[50];
> > +
> > +  CHAR8 Str[20];
> > +
> > +  UINT8 TempByte;
> > +  UINTN Size;
> > +  UINTN Index;
> > +
> > +  Data = UserData;
> > +  while (DataSize != 0) {
> > +    Size = 16;
> > +    if (Size > DataSize) {
> > +      Size = DataSize;
> > +    }
> > +
> > +    for (Index = 0; Index < Size; Index += 1) {
> > +      TempByte            = Data[Index];
> > +      Val[Index * 3 + 0]  = mHex[TempByte >> 4];
> > +      Val[Index * 3 + 1]  = mHex[TempByte & 0xF];
> > +      Val[Index * 3 + 2]  = (CHAR8) ((Index == 7) ? '-' : ' ');
> > +      Str[Index]          = (CHAR8) ((TempByte < ' ' || TempByte > 'z') ? '.' :
> > TempByte);
> > +    }
> > +
> > +    Val[Index * 3]  = 0;
> > +    Str[Index]      = 0;
> > +    Print (L"%*a%08X: %-48a *%a*\r\n", Indent, "", Offset, Val, Str);
> > +
> > +    Data += Size;
> > +    Offset += Size;
> > +    DataSize -= Size;
> > +  }
> > +}
> > +
> > +
> > +/**
> > +  Safely append with automatic string resizing given length of
> > +Destination and
> > +  desired length of copy from Source.
> > +
> > +  append the first D characters of Source to the end of Destination,
> > + where D is  the lesser of Count and the StrLen() of Source. If
> > + appending those D
> > characters
> > +  will fit within Destination (whose Size is given as CurrentSize)
> > + and  still leave room for a NULL terminator, then those characters
> > + are appended,  starting at the original terminating NULL of
> > + Destination, and a new
> > terminating
> > +  NULL is appended.
> > +
> > +  If appending D characters onto Destination will result in a
> > + overflow of the
> > size
> > +  given in CurrentSize the string will be grown such that the copy
> > + can be
> > performed
> > +  and CurrentSize will be updated to the new size.
> > +
> > +  If Source is NULL, there is nothing to append, just return the
> > + current buffer in  Destination.
> > +
> > +  if Destination is NULL, then ASSERT()  if Destination's current
> > + length (including NULL terminator) is already more
> > then
> > +  CurrentSize, then ASSERT()
> > +
> > +  @param[in, out] Destination   The String to append onto
> > +  @param[in, out] CurrentSize   on call the number of bytes in Destination.
> On
> > +                                return possibly the new size (still in bytes).  if NULL
> > +                                then allocate whatever is needed.
> > +  @param[in]      Source        The String to append from
> > +  @param[in]      Count         Maximum number of characters to append.  if 0
> > then
> > +                                all are appended.
> > +
> > +  @return Destination           return the resultant string.
> > +**/
> > +CHAR16*
> > +EFIAPI
> > +InternalStrnCatGrow (
> 
> This one is an application internal function, 'EFIAPI' is not needed here.
> 
> > +  IN OUT CHAR16           **Destination,
> > +  IN OUT UINTN            *CurrentSize,
> > +  IN     CONST CHAR16     *Source,
> > +  IN     UINTN            Count
> 
> Seems to me that the 'Count' parameter is always set to 0 among the calls
> within this proposed application. Maybe it is better to just eliminate it.
> 
> > +  )
> > +{
> > +  UINTN DestinationStartSize;
> > +  UINTN NewSize;
> > +
> > +  //
> > +  // ASSERTs
> > +  //
> > +  ASSERT(Destination != NULL);
> > +
> > +  //
> > +  // If there's nothing to do then just return Destination  //  if
> > + (Source == NULL) {
> > +    return (*Destination);
> > +  }
> > +
> > +  //
> > +  // allow for un-initialized pointers, based on size being 0  //  if
> > + (CurrentSize != NULL && *CurrentSize == 0) {
> > +    *Destination = NULL;
> > +  }
> > +
> > +  //
> > +  // allow for NULL pointers address as Destination
> > +  //
> > +  if (*Destination != NULL) {
> > +    ASSERT(CurrentSize != 0);
> > +    DestinationStartSize = StrSize(*Destination);
> > +    ASSERT(DestinationStartSize <= *CurrentSize);
> > +  } else {
> > +    DestinationStartSize = 0;
> > +//    ASSERT(*CurrentSize == 0);
> > +  }
> > +
> > +  //
> > +  // Append all of Source?
> > +  //
> > +  if (Count == 0) {
> > +    Count = StrLen(Source);
> > +  }
> > +
> > +  //
> > +  // Test and grow if required
> > +  //
> > +  if (CurrentSize != NULL) {
> > +    NewSize = *CurrentSize;
> > +    if (NewSize < DestinationStartSize + (Count * sizeof(CHAR16))) {
> > +      while (NewSize < (DestinationStartSize + (Count*sizeof(CHAR16)))) {
> > +        NewSize += 2 * Count * sizeof(CHAR16);
> > +      }
> > +      *Destination = ReallocatePool(*CurrentSize, NewSize, *Destination);
> > +      *CurrentSize = NewSize;
> > +    }
> > +  } else {
> > +    NewSize = (Count+1)*sizeof(CHAR16);
> > +    *Destination = AllocateZeroPool(NewSize);  }
> > +
> > +  //
> > +  // Now use standard StrnCat on a big enough buffer  //  if
> > + (*Destination == NULL) {
> > +    return (NULL);
> > +  }
> > +
> > +  StrnCatS(*Destination, NewSize/sizeof(CHAR16), Source, Count);
> > +  return *Destination;
> > +}
> > +
> > +/**
> > +  Get PCD type string based on input PCD type.
> > +
> > +  @param[in]    TokenSpace      PCD Token Space.
> > +  @param[in]    PcdType         The input PCD type.
> > +
> > +  @return       Pointer to PCD type string.
> > +**/
> > +CHAR16 *
> > +GetPcdTypeString (
> > +  IN CONST EFI_GUID     *TokenSpace,
> > +  IN EFI_PCD_TYPE       PcdType
> > +  )
> > +{
> > +  UINTN  BufLen;
> > +  CHAR16 *RetString;
> > +
> > +  BufLen      = 0;
> > +  RetString   = NULL;
> > +
> > +  switch (PcdType) {
> > +    case EFI_PCD_TYPE_8:
> > +      InternalStrnCatGrow (&RetString, &BufLen, L"UINT8", 0);
> > +      break;
> > +    case EFI_PCD_TYPE_16:
> > +      InternalStrnCatGrow (&RetString, &BufLen, L"UINT16", 0);
> > +      break;
> > +    case EFI_PCD_TYPE_32:
> > +      InternalStrnCatGrow (&RetString, &BufLen, L"UINT32", 0);
> > +      break;
> > +    case EFI_PCD_TYPE_64:
> > +      InternalStrnCatGrow (&RetString, &BufLen, L"UINT64", 0);
> > +      break;
> > +    case EFI_PCD_TYPE_BOOL:
> > +      InternalStrnCatGrow (&RetString, &BufLen, L"BOOLEAN", 0);
> > +      break;
> > +    case EFI_PCD_TYPE_PTR:
> > +      InternalStrnCatGrow (&RetString, &BufLen, L"POINTER", 0);
> > +      break;
> > +    default:
> > +      InternalStrnCatGrow (&RetString, &BufLen, L"UNKNOWN", 0);
> > +      break;
> > +  }
> > +
> > +  if (TokenSpace == NULL) {
> > +    InternalStrnCatGrow (&RetString, &BufLen, L":DYNAMIC", 0);  }
> > + else {
> > +    InternalStrnCatGrow (&RetString, &BufLen, L":DYNAMICEX", 0);  }
> > +
> > +  return RetString;
> > +}
> > +
> > +/**
> > +  Dump PCD info.
> > +
> > +  @param[in]    TokenSpace      PCD Token Space.
> > +  @param[in]    TokenNumber     PCD Token Number.
> > +  @param[in]    PcdInfo         Pointer to PCD info.
> > +**/
> > +VOID
> > +DumpPcdInfo (
> > +  IN CONST EFI_GUID     *TokenSpace,
> > +  IN UINTN              TokenNumber,
> > +  IN EFI_PCD_INFO       *PcdInfo
> > +  )
> > +{
> > +  CHAR16                *RetString;
> > +  UINT8                 Uint8;
> > +  UINT16                Uint16;
> > +  UINT32                Uint32;
> > +  UINT64                Uint64;
> > +  BOOLEAN               Boolean;
> > +  VOID                  *PcdData;
> > +
> > +  RetString = NULL;
> > +
> > +  if (PcdInfo->PcdName != NULL) {
> > +    Print (L"%a\n", PcdInfo->PcdName);  } else {
> > +    if (TokenSpace == NULL) {
> > +      Print (L"Default Token Space\n");
> > +    } else {
> > +      Print (L"%g\n", TokenSpace);
> > +    }
> > +  }
> > +
> > +  RetString = GetPcdTypeString (TokenSpace, PcdInfo->PcdType);
> > +
> > +  switch (PcdInfo->PcdType) {
> > +    case EFI_PCD_TYPE_8:
> > +      if (TokenSpace == NULL) {
> > +        Uint8 = mPcd->Get8 (TokenNumber);
> > +      } else {
> > +        Uint8 = mPiPcd->Get8 (TokenSpace, TokenNumber);
> > +      }
> > +      Print (L"  Token = 0x%08x - Type = %H%-17s%N - Size = 0x%x -
> > + Value =
> > 0x%x\n", TokenNumber, RetString, PcdInfo->PcdSize, Uint8);
> > +      break;
> > +    case EFI_PCD_TYPE_16:
> > +      if (TokenSpace == NULL) {
> > +        Uint16 = mPcd->Get16 (TokenNumber);
> > +      } else {
> > +        Uint16 = mPiPcd->Get16 (TokenSpace, TokenNumber);
> > +      }
> > +      Print (L"  Token = 0x%08x - Type = %H%-17s%N - Size = 0x%x -
> > + Value =
> > 0x%x\n", TokenNumber, RetString, PcdInfo->PcdSize, Uint16);
> > +      break;
> > +    case EFI_PCD_TYPE_32:
> > +      if (TokenSpace == NULL) {
> > +        Uint32 = mPcd->Get32 (TokenNumber);
> > +      } else {
> > +        Uint32 = mPiPcd->Get32 (TokenSpace, TokenNumber);
> > +      }
> > +      Print (L"  Token = 0x%08x - Type = %H%-17s%N - Size = 0x%x -
> > + Value =
> > 0x%x\n", TokenNumber, RetString, PcdInfo->PcdSize, Uint32);
> > +      break;
> > +    case EFI_PCD_TYPE_64:
> > +      if (TokenSpace == NULL) {
> > +        Uint64 = mPcd->Get64 (TokenNumber);
> > +      } else {
> > +        Uint64 = mPiPcd->Get64 (TokenSpace, TokenNumber);
> > +      }
> > +      Print (L"  Token = 0x%08x - Type = %H%-17s%N - Size = 0x%x -
> > + Value =
> > 0x%lx\n", TokenNumber, RetString, PcdInfo->PcdSize, Uint64);
> > +      break;
> > +    case EFI_PCD_TYPE_BOOL:
> > +      if (TokenSpace == NULL) {
> > +        Boolean = mPcd->GetBool (TokenNumber);
> > +      } else {
> > +        Boolean = mPiPcd->GetBool (TokenSpace, TokenNumber);
> > +      }
> > +      Print (L"  Token = 0x%08x - Type = %H%-17s%N - Size = 0x%x -
> > + Value
> > = %a\n", TokenNumber, RetString, PcdInfo->PcdSize, Boolean ? "TRUE" :
> > "FALSE");
> > +      break;
> > +    case EFI_PCD_TYPE_PTR:
> > +      if (TokenSpace == NULL) {
> > +        PcdData = mPcd->GetPtr (TokenNumber);
> > +      } else {
> > +        PcdData = mPiPcd->GetPtr (TokenSpace, TokenNumber);
> > +      }
> > +      Print (L"  Token = 0x%08x - Type = %H%-17s%N - Size = 0x%x\n",
> > TokenNumber, RetString, PcdInfo->PcdSize);
> > +      DumpHex (2, 0, PcdInfo->PcdSize, PcdData);
> > +      break;
> > +    default:
> > +      return;
> > +  }
> > +
> > +  if (RetString != NULL) {
> > +    FreePool (RetString);
> > +  }
> > +  Print (L"\n");
> > +}
> > +
> > +/**
> > +  Show one or all PCDs' info.
> > +
> > +  @param[in]  InputPcdName       Pointer to PCD name to show. If NULL,
> show
> > all PCDs' info.
> > +
> > +  @retval EFI_SUCCESS            Command completed successfully.
> > +  @retval EFI_OUT_OF_RESOURCES   Not enough resources were available
> to
> > run the command.
> > +  @retval EFI_ABORTED            Aborted by user.
> > +  @retval EFI_NOT_FOUND          The specified PCD is not found.
> > +**/
> > +EFI_STATUS
> > +ProcessPcd (
> > +  IN CHAR16     *InputPcdName
> > +  )
> > +{
> > +  EFI_STATUS            Status;
> > +  EFI_GUID              *TokenSpace;
> > +  UINTN                 TokenNumber;
> > +  EFI_PCD_INFO          PcdInfo;
> > +  BOOLEAN               Found;
> > +  UINTN                 PcdNameSize;
> > +
> > +  PcdInfo.PcdName = NULL;
> > +  PcdInfo.PcdSize = 0;
> > +  PcdInfo.PcdType = 0xFF;
> > +  Found = FALSE;
> > +
> > +  Print (L"Current system SKU ID: 0x%x\n\n", mPiPcdInfo->GetSku ());
> > +
> > +  TokenSpace = NULL;
> > +  do {
> > +    TokenNumber = 0;
> > +    do {
> > +      Status = mPiPcd->GetNextToken (TokenSpace, &TokenNumber);
> > +      if (!EFI_ERROR (Status) && TokenNumber != 0) {
> > +        if (TokenSpace == NULL) {
> > +          //
> > +          // PCD in default Token Space.
> > +          //
> > +          mPcdInfo->GetInfo (TokenNumber, &PcdInfo);
> > +        } else {
> > +          mPiPcdInfo->GetInfo (TokenSpace, TokenNumber, &PcdInfo);
> > +        }
> > +        if (InputPcdName != NULL) {
> > +          if (PcdInfo.PcdName == NULL) {
> > +            continue;
> > +          }
> > +          PcdNameSize = AsciiStrSize (PcdInfo.PcdName) * sizeof (CHAR16);
> > +          if (mTempPcdNameBuffer == NULL) {
> > +            mTempPcdNameBufferSize = PcdNameSize;
> > +            mTempPcdNameBuffer = AllocatePool
> (mTempPcdNameBufferSize);
> > +          } else if (mTempPcdNameBufferSize < PcdNameSize) {
> > +            mTempPcdNameBuffer = ReallocatePool
> > + (mTempPcdNameBufferSize,
> > PcdNameSize, mTempPcdNameBuffer);
> > +            mTempPcdNameBufferSize = PcdNameSize;
> > +          }
> > +          if (mTempPcdNameBuffer == NULL) {
> > +            return EFI_OUT_OF_RESOURCES;
> > +          }
> > +          AsciiStrToUnicodeStrS (PcdInfo.PcdName, mTempPcdNameBuffer,
> > mTempPcdNameBufferSize / sizeof (CHAR16));
> > +          //
> > +          // Compare the input PCD name with the PCD name in PCD database.
> > +          //
> > +          if ((StrStr (mTempPcdNameBuffer, InputPcdName) != NULL) ||
> > +              (mUnicodeCollation != NULL &&
> > + mUnicodeCollation->MetaiMatch
> > (mUnicodeCollation, mTempPcdNameBuffer, InputPcdName))) {
> > +            //
> > +            // Found matched PCD.
> > +            //
> > +            DumpPcdInfo (TokenSpace, TokenNumber, &PcdInfo);
> > +            Found = TRUE;
> > +          }
> > +        } else {
> > +          DumpPcdInfo (TokenSpace, TokenNumber, &PcdInfo);
> > +        }
> > +      }
> > +    } while (!EFI_ERROR (Status) && TokenNumber != 0);
> > +
> > +    Status = mPiPcd->GetNextTokenSpace ((CONST EFI_GUID **)
> > + &TokenSpace);  } while (!EFI_ERROR (Status) && TokenSpace != NULL);
> > +
> > +  if ((InputPcdName != NULL) && !Found) {
> > +    //
> > +    // The specified PCD is not found, print error.
> > +    //
> > +    Print (L"%EError. %NNo matching PCD found: %s.\n", InputPcdName);
> > +    return EFI_NOT_FOUND;
> > +  }
> > +  return EFI_SUCCESS;
> > +}
> > +
> > +/**
> > +  Main entrypoint for DumpDynPcd shell application.
> > +
> > +  @param[in]  ImageHandle     The image handle.
> > +  @param[in]  SystemTable     The system table.
> > +
> > +  @retval EFI_SUCCESS            Command completed successfully.
> > +  @retval EFI_INVALID_PARAMETER  Command usage error.
> > +  @retval EFI_OUT_OF_RESOURCES   Not enough resources were available
> to
> > run the command.
> > +  @retval EFI_ABORTED            Aborted by user.
> > +  @retval EFI_NOT_FOUND          The specified PCD is not found.
> > +  @retval Others                 Error status returned from gBS->LocateProtocol.
> > +**/
> > +EFI_STATUS
> > +EFIAPI
> > +DumpDynPcdMain (
> > +  IN EFI_HANDLE        ImageHandle,
> > +  IN EFI_SYSTEM_TABLE  *SystemTable
> > +  )
> > +{
> > +  EFI_STATUS    Status;
> > +  CHAR16        *InputPcdName;
> > +
> > +  InputPcdName  = NULL;
> > +
> > +  Status = gBS->LocateProtocol(&gEfiUnicodeCollation2ProtocolGuid,
> > + NULL,
> > (VOID **) &mUnicodeCollation);
> > +  if (EFI_ERROR (Status)) {
> > +    mUnicodeCollation = NULL;
> > +  }
> > +
> > +  Status = gBS->LocateProtocol (&gEfiPcdProtocolGuid, NULL, (VOID **)
> > &mPiPcd);
> > +  if (EFI_ERROR (Status)) {
> > +    Print (L"DumpDynPcd: %EError. %NPI PCD protocol is not present.\n");
> > +    return Status;
> > +  }
> > +
> > +  Status = gBS->LocateProtocol (&gEfiGetPcdInfoProtocolGuid, NULL,
> > + (VOID **)
> > &mPiPcdInfo);
> > +  if (EFI_ERROR (Status)) {
> > +    Print (L"DumpDynPcd: %EError. %NPI PCD info protocol is not
> present.\n");
> > +    return Status;
> > +  }
> > +
> > +  Status = gBS->LocateProtocol (&gPcdProtocolGuid, NULL, (VOID **)
> > + &mPcd);  if (EFI_ERROR (Status)) {
> > +    Print (L"DumpDynPcd: %EError. %NPCD protocol is not present.\n");
> > +    return Status;
> > +  }
> > +
> > +  Status = gBS->LocateProtocol (&gGetPcdInfoProtocolGuid, NULL, (VOID
> > + **)
> > &mPcdInfo);
> > +  if (EFI_ERROR (Status)) {
> > +    Print (L"DumpDynPcd: %EError. %NPCD info protocol is not present.\n");
> > +    return Status;
> > +  }
> > +
> > +  //
> > +  // get the command line arguments
> > +  //
> > +  Status = GetArg();
> > +  if (EFI_ERROR(Status)){
> > +    Print (L"DumpDynPcd: %EError. %NThe input parameters are not
> > recognized.\n");
> > +    Status = EFI_INVALID_PARAMETER;
> > +    return Status;
> > +  }
> > +
> > +  if (Argc > 2){
> > +    Print (L"DumpDynPcd: %EError. %NToo many arguments specified.\n");
> > +    Status = EFI_INVALID_PARAMETER;
> > +    return Status;
> > +  }
> > +
> > +  if (Argc == 1){
> > +    Status = ProcessPcd (InputPcdName);
> > +    goto Done;
> > +  }
> > +
> > +  if ((StrCmp(Argv[1], L"-?") == 0)||(StrCmp(Argv[1], L"-h") ==
> > 0)||(StrCmp(Argv[1], L"-H") == 0)){
> > +    ShowHelp ();
> > +    goto Done;
> > +  } else
> 
> Please put an open brace after 'else'.
> 
> Seems to me there is a hard requirement according to EDK II C Coding
> Standards Specification Version 2.1, Section 5.7.3.4.2:
> 
> '''
> An open brace always follows the else
> '''
> 
> > +    if ((StrCmp(Argv[1], L"-v") == 0)||(StrCmp(Argv[1], L"-V") == 0)){
> > +      ShowVersion ();
> > +      goto Done;
> > +    } else
> 
> Similar comment as above.
> 
> > +      if (StrStr(Argv[1], L"-") != NULL){
> > +        Print (L"DumpDynPcd: %EError. %NThe argument '%B%s%N' is
> > + invalid.\n",
> > Argv[1]);
> > +        goto Done;
> > +      }
> > +
> > +  InputPcdName = Argv[1];
> > +  Status = ProcessPcd (InputPcdName);
> > +
> > +  Done:
> > +
> > +  if (mTempPcdNameBuffer != NULL) {
> > +    FreePool (mTempPcdNameBuffer);
> > +  }
> > +
> > +  return Status;
> > +}
> > +
> > diff --git a/MdeModulePkg/Application/DumpDynPcd/DumpDynPcd.inf
> > b/MdeModulePkg/Application/DumpDynPcd/DumpDynPcd.inf
> > new file mode 100644
> > index 0000000000..af63eccca7
> > --- /dev/null
> > +++ b/MdeModulePkg/Application/DumpDynPcd/DumpDynPcd.inf
> > @@ -0,0 +1,51 @@
> > +#
> > +#  DumpDynPcd is a shell application to dump dynamic pcd information.
> > +#
> > +#  Copyright (c) 2019, Intel Corporation. All rights reserved.<BR> #
> > +SPDX-License-Identifier: BSD-2-Clause-Patent # ##
> > +
> > +[Defines]
> > +  INF_VERSION                    = 0x00010006
> > +  BASE_NAME                      = DumpDynPcd
> > +  FILE_GUID                      = 31ADA2B2-62EA-4866-9B87-03FEA8425974
> > +  MODULE_TYPE                    = UEFI_APPLICATION
> > +  VERSION_STRING                 = 1.0
> > +  ENTRY_POINT                    = DumpDynPcdMain
> > +
> > +#
> > +# This flag specifies whether HII resource section is generated into PE
> image.
> > +#
> > +  UEFI_HII_RESOURCE_SECTION      = TRUE
> > +
> > +#
> > +# The following information is for reference only and not required by
> > +the build
> > tools.
> > +#
> > +#  VALID_ARCHITECTURES           = IA32 X64 IPF EBC
> 
> Please help to remove 'IPF' in the above comment.
> 
> > +#
> > +
> > +[Sources]
> > +  DumpDynPcd.c
> > +  DumpDynPcdStr.uni
> > +
> > +[Packages]
> > +  MdePkg/MdePkg.dec
> > +  ShellPkg/ShellPkg.dec
> 
> Please help to drop the ShellPkg.dec here.
> I do not think there is any library/protocol dependency in ShellPkg.
> 
> Best Regards,
> Hao Wu
> 
> > +
> > +[LibraryClasses]
> > +  BaseLib
> > +  UefiApplicationEntryPoint
> > +  DebugLib
> > +  MemoryAllocationLib
> > +  UefiLib
> > +  UefiBootServicesTableLib
> > +
> > +[Protocols]
> > +  gEfiUnicodeCollation2ProtocolGuid     ## SOMETIMES_CONSUMES
> > +  gEfiPcdProtocolGuid                   ## CONSUMES
> > +  gPcdProtocolGuid                      ## CONSUMES
> > +  gEfiGetPcdInfoProtocolGuid            ## CONSUMES
> > +  gGetPcdInfoProtocolGuid               ## CONSUMES
> > +  gEfiShellParametersProtocolGuid       ## CONSUMES
> > +
> > diff --git a/MdeModulePkg/Application/DumpDynPcd/DumpDynPcdStr.uni
> > b/MdeModulePkg/Application/DumpDynPcd/DumpDynPcdStr.uni
> > new file mode 100644
> > index 0000000000..f0ee98219b
> > --- /dev/null
> > +++ b/MdeModulePkg/Application/DumpDynPcd/DumpDynPcdStr.uni
> > @@ -0,0 +1,28 @@
> > +//
> > +// DumpDynPcd is a shell application to dump dynamic pcd information.
> > +//
> > +// Copyright (c) 2019, Intel Corporation. All rights reserved.<BR> //
> > +SPDX-License-Identifier: BSD-2-Clause-Patent // //**/
> > +
> > +/=#
> > +
> > +#langdef en-US "English"
> > +
> > +#string STR_DUMP_DYN_PCD_HELP_INFORMATION       #language en-US
> ""
> > +                                                                ".TH
> > +DumpDynPcd 0 "Dump
> > dynamic[ex] PCD info."\r\n"
> > +                                                                ".SH NAME\r\n"
> > +                                                                "Dump dynamic[ex] PCD info.\r\n"
> > +                                                                ".SH SYNOPSIS\r\n"
> > +                                                                " \r\n"
> > +                                                                "DumpDynPcd [PcdName].\r\n"
> > +                                                                ".SH OPTIONS\r\n"
> > +                                                                " \r\n"
> > +                                                                "  PcdName    Specifies the name of
> > PCD.\r\n"
> > +                                                                "             A literal[or partial] name or a
> > pattern as specified in\r\n"
> > +                                                                "             the MetaiMatch() function of
> > the EFI_UNICODE_COLLATION2_PROCOOL.\r\n"
> > +                                                                "             If it is absent, dump all PCDs'
> > info.\r\n"
> > +                                                                "The
> > + PCD data is printed as
> > hexadecimal dump.\n"
> > +                                                                "\r\n"
> > +
> > diff --git a/MdeModulePkg/MdeModulePkg.dsc
> > b/MdeModulePkg/MdeModulePkg.dsc index b302f4a4f3..a8b9d8d027
> 100644
> > --- a/MdeModulePkg/MdeModulePkg.dsc
> > +++ b/MdeModulePkg/MdeModulePkg.dsc
> > @@ -206,6 +206,7 @@
> >
> >  [Components]
> >    MdeModulePkg/Application/HelloWorld/HelloWorld.inf
> > +  MdeModulePkg/Application/DumpDynPcd/DumpDynPcd.inf
> >    MdeModulePkg/Application/MemoryProfileInfo/MemoryProfileInfo.inf
> >
> >    MdeModulePkg/Bus/Pci/PciHostBridgeDxe/PciHostBridgeDxe.inf
> > --
> > 2.18.0.windows.1