[edk2-devel] [PATCH] MdeModulePkg/DumpDynPcd: Add a new application to dump dynamic PCD settings
Dong, Eric
eric.dong at intel.com
Tue May 7 00:25:39 UTC 2019
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 at intel.com>; Dong, Eric
> <eric.dong at intel.com>; devel at edk2.groups.io
> Cc: Wang, Jian J <jian.j.wang at intel.com>; Ni, Ray <ray.ni at intel.com>; Zeng,
> Star <star.zeng at 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 at 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 at intel.com>
> > Cc: Hao Wu <hao.a.wu at intel.com>
> > Cc: Ray Ni <ray.ni at intel.com>
> > Cc: Star Zeng <star.zeng at intel.com>
> > Signed-off-by: Shenglei Zhang <shenglei.zhang at 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
-=-=-=-=-=-=-=-=-=-=-=-
Groups.io Links: You receive all messages sent to this group.
View/Reply Online (#40055): https://edk2.groups.io/g/devel/message/40055
Mute This Topic: https://groups.io/mt/31506803/1813853
Group Owner: devel+owner at edk2.groups.io
Unsubscribe: https://edk2.groups.io/g/devel/unsub [edk2-devel-archive at redhat.com]
-=-=-=-=-=-=-=-=-=-=-=-
More information about the edk2-devel-archive
mailing list