[PATCH v1 0/5] Fix imbalanced debug macros

[PATCH v1 0/5] Fix imbalanced debug macros

[Michael Kubacki]

From: Michael Kubacki <michael.kubacki@microsoft.com>

After noticing a few occurrences of DEBUG macros with a mismatched
number of arguments, I wrote a script to automatically detect this.

This patch series is the result of matches found by the script.

I would like to eventually check the script in as a build plugin
but that will require a bit more finishing touches and testing.

For now, this series fixes the problems.

Cc: Abner Chang <abner.chang@amd.com>
Cc: Ard Biesheuvel <ardb+tianocore@kernel.org>
Cc: Dandan Bi <dandan.bi@intel.com>
Cc: Guomin Jiang <guomin.jiang@intel.com>
Cc: Hao A Wu <hao.a.wu@intel.com>
Cc: Jian J Wang <jian.j.wang@intel.com>
Cc: Jiewen Yao <jiewen.yao@intel.com>
Cc: Leif Lindholm <quic_llindhol@quicinc.com>
Cc: Liming Gao <gaoliming@byosoft.com.cn>
Cc: Nickle Wang <nickle.wang@hpe.com>
Cc: Qi Zhang <qi1.zhang@intel.com>
Cc: Rahul Kumar <rahul1.kumar@intel.com>
Cc: Ray Ni <ray.ni@intel.com>
Signed-off-by: Michael Kubacki <michael.kubacki@microsoft.com>

Michael Kubacki (5):
  ArmPlatformPkg/NorFlashDxe: Remove unused debug print specifier
  FatPkg/FatPei: Remove extraneous debug message argument
  MdeModulePkg: Fix imbalanced debug macros
  RedfishPkg/RedfishRestExDxe: Remove extra debug macro argument
  SecurityPkg/SmmTcg2PhysicalPresenceLib: Add missing debug print
    specifier

 ArmPlatformPkg/Drivers/NorFlashDxe/NorFlashBlockIoDxe.c                                  | 4 ++--
 FatPkg/FatPei/Gpt.c                                                                      | 2 +-
 MdeModulePkg/Bus/Pci/SdMmcPciHcDxe/SdMmcPciHci.c                                         | 2 +-
 MdeModulePkg/Bus/Usb/UsbBusDxe/UsbEnumer.c                                               | 8 ++++----
 MdeModulePkg/Core/Dxe/Image/Image.c                                                      | 2 +-
 MdeModulePkg/Library/DisplayUpdateProgressLibGraphics/DisplayUpdateProgressLibGraphics.c | 2 +-
 RedfishPkg/RedfishRestExDxe/RedfishRestExProtocol.c                                      | 2 +-
 SecurityPkg/Library/SmmTcg2PhysicalPresenceLib/MmTcg2PhysicalPresenceLibCommon.c         | 2 +-
 8 files changed, 12 insertions(+), 12 deletions(-)

-- 
2.28.0.windows.1

[PATCH v1 1/5] ArmPlatformPkg/NorFlashDxe: Remove unused debug print specifier

[Michael Kubacki]

From: Michael Kubacki <michael.kubacki@microsoft.com>

These debug messages are repeated in both NorFlashBlockIoReadBlocks()
and NorFlashBlockIoWriteBlocks():

  "NorFlashBlockIoWriteBlocks(MediaId=0x%x, Lba=%ld, BufferSize=0x%x"
  "bytes (%d kB), BufferPtr @ 0x%08x)\n"

Although this requires 5 arguments, only 4 are provided. The kilobyte
value was never given.

This change removes that specifier so the 4 arguments match the 4
specifiers in the debug macro.

Cc: Leif Lindholm <quic_llindhol@quicinc.com>
Cc: Ard Biesheuvel <ardb+tianocore@kernel.org>
Signed-off-by: Michael Kubacki <michael.kubacki@microsoft.com>
---
 ArmPlatformPkg/Drivers/NorFlashDxe/NorFlashBlockIoDxe.c | 4 ++--
 1 file changed, 2 insertions(+), 2 deletions(-)

diff --git a/ArmPlatformPkg/Drivers/NorFlashDxe/NorFlashBlockIoDxe.c b/ArmPlatformPkg/Drivers/NorFlashDxe/NorFlashBlockIoDxe.c
index 5afab0a79fa2..e671108e2bcf 100644
--- a/ArmPlatformPkg/Drivers/NorFlashDxe/NorFlashBlockIoDxe.c
+++ b/ArmPlatformPkg/Drivers/NorFlashDxe/NorFlashBlockIoDxe.c
@@ -54,7 +54,7 @@ NorFlashBlockIoReadBlocks (
   Instance = INSTANCE_FROM_BLKIO_THIS (This);
   Media    = This->Media;
 
-  DEBUG ((DEBUG_BLKIO, "NorFlashBlockIoReadBlocks(MediaId=0x%x, Lba=%ld, BufferSize=0x%x bytes (%d kB), BufferPtr @ 0x%08x)\n", MediaId, Lba, BufferSizeInBytes, Buffer));
+  DEBUG ((DEBUG_BLKIO, "NorFlashBlockIoReadBlocks(MediaId=0x%x, Lba=%ld, BufferSize=0x%x bytes, BufferPtr @ 0x%08x)\n", MediaId, Lba, BufferSizeInBytes, Buffer));
 
   if (!Media) {
     Status = EFI_INVALID_PARAMETER;
@@ -89,7 +89,7 @@ NorFlashBlockIoWriteBlocks (
 
   Instance = INSTANCE_FROM_BLKIO_THIS (This);
 
-  DEBUG ((DEBUG_BLKIO, "NorFlashBlockIoWriteBlocks(MediaId=0x%x, Lba=%ld, BufferSize=0x%x bytes (%d kB), BufferPtr @ 0x%08x)\n", MediaId, Lba, BufferSizeInBytes, Buffer));
+  DEBUG ((DEBUG_BLKIO, "NorFlashBlockIoWriteBlocks(MediaId=0x%x, Lba=%ld, BufferSize=0x%x bytes, BufferPtr @ 0x%08x)\n", MediaId, Lba, BufferSizeInBytes, Buffer));
 
   if ( !This->Media->MediaPresent ) {
     Status = EFI_NO_MEDIA;
-- 
2.28.0.windows.1

[PATCH v1 2/5] FatPkg/FatPei: Remove extraneous debug message argument

[Michael Kubacki]

From: Michael Kubacki <michael.kubacki@microsoft.com>

This debug macro should take one argument based on the number of
print specifiers defined. However, two arguments are given.

It looks like the code may have been refactored such that the
second argument was moved to a new print and this argument was
not removed. In any case, it should not be there now.

Cc: Ray Ni <ray.ni@intel.com>
Signed-off-by: Michael Kubacki <michael.kubacki@microsoft.com>
---
 FatPkg/FatPei/Gpt.c | 2 +-
 1 file changed, 1 insertion(+), 1 deletion(-)

diff --git a/FatPkg/FatPei/Gpt.c b/FatPkg/FatPei/Gpt.c
index 0a1a25ceeff1..9e17ce88086f 100644
--- a/FatPkg/FatPei/Gpt.c
+++ b/FatPkg/FatPei/Gpt.c
@@ -361,7 +361,7 @@ PartitionCheckGptEntryArray (
 
     PrivateData->BlockDeviceCount++;
 
-    DEBUG ((DEBUG_INFO, "Find GPT Partition [0x%lx", PartitionEntryBuffer[Index].StartingLBA, BlockDevPtr->LastBlock));
+    DEBUG ((DEBUG_INFO, "Find GPT Partition [0x%lx", PartitionEntryBuffer[Index].StartingLBA));
     DEBUG ((DEBUG_INFO, ", 0x%lx]\n", BlockDevPtr->LastBlock));
     DEBUG ((DEBUG_INFO, "         BlockSize %x\n", BlockDevPtr->BlockSize));
   }
-- 
2.28.0.windows.1

[PATCH v1 3/5] MdeModulePkg: Fix imbalanced debug macros

[Michael Kubacki]

From: Michael Kubacki <michael.kubacki@microsoft.com>

Updates debug macros in the package that have an imbalanced number
of print specifiers to arguments. These changes try to preserve
what was likely intended by the author. In cases information was
missing due to the bug, the specifier may be removed since it was
not previously accurately printing the expected value.

Cc: Dandan Bi <dandan.bi@intel.com>
Cc: Guomin Jiang <guomin.jiang@intel.com>
Cc: Hao A Wu <hao.a.wu@intel.com>
Cc: Jian J Wang <jian.j.wang@intel.com>
Cc: Liming Gao <gaoliming@byosoft.com.cn>
Cc: Liming Gao <gaoliming@byosoft.com.cn>
Cc: Ray Ni <ray.ni@intel.com>
Signed-off-by: Michael Kubacki <michael.kubacki@microsoft.com>
---
 MdeModulePkg/Bus/Pci/SdMmcPciHcDxe/SdMmcPciHci.c                                         | 2 +-
 MdeModulePkg/Bus/Usb/UsbBusDxe/UsbEnumer.c                                               | 8 ++++----
 MdeModulePkg/Core/Dxe/Image/Image.c                                                      | 2 +-
 MdeModulePkg/Library/DisplayUpdateProgressLibGraphics/DisplayUpdateProgressLibGraphics.c | 2 +-
 4 files changed, 7 insertions(+), 7 deletions(-)

diff --git a/MdeModulePkg/Bus/Pci/SdMmcPciHcDxe/SdMmcPciHci.c b/MdeModulePkg/Bus/Pci/SdMmcPciHcDxe/SdMmcPciHci.c
index 53b63ab52b93..dd45167a009e 100644
--- a/MdeModulePkg/Bus/Pci/SdMmcPciHcDxe/SdMmcPciHci.c
+++ b/MdeModulePkg/Bus/Pci/SdMmcPciHcDxe/SdMmcPciHci.c
@@ -64,7 +64,7 @@ DumpCapabilityReg (
   DEBUG ((DEBUG_INFO, "   Driver Type D     %a\n", Capability->DriverTypeD ? "TRUE" : "FALSE"));
   DEBUG ((DEBUG_INFO, "   Driver Type 4     %a\n", Capability->DriverType4 ? "TRUE" : "FALSE"));
   if (Capability->TimerCount == 0) {
-    DEBUG ((DEBUG_INFO, "   Retuning TimerCnt Disabled\n", 2 * (Capability->TimerCount - 1)));
+    DEBUG ((DEBUG_INFO, "   Retuning TimerCnt Disabled\n"));
   } else {
     DEBUG ((DEBUG_INFO, "   Retuning TimerCnt %dseconds\n", 2 * (Capability->TimerCount - 1)));
   }
diff --git a/MdeModulePkg/Bus/Usb/UsbBusDxe/UsbEnumer.c b/MdeModulePkg/Bus/Usb/UsbBusDxe/UsbEnumer.c
index 5495b324b381..aed34596f469 100644
--- a/MdeModulePkg/Bus/Usb/UsbBusDxe/UsbEnumer.c
+++ b/MdeModulePkg/Bus/Usb/UsbBusDxe/UsbEnumer.c
@@ -941,7 +941,7 @@ UsbEnumeratePort (
       //   which probably is caused by short circuit. It has to wait system hardware
       //   to perform recovery.
       //
-      DEBUG ((DEBUG_ERROR, "UsbEnumeratePort: Critical Over Current\n", Port));
+      DEBUG ((DEBUG_ERROR, "UsbEnumeratePort: Critical Over Current (port %d)\n", Port));
       return EFI_DEVICE_ERROR;
     }
 
@@ -951,7 +951,7 @@ UsbEnumeratePort (
     //   over current. As a result, all ports are nearly power-off, so
     //   it's necessary to detach and enumerate all ports again.
     //
-    DEBUG ((DEBUG_ERROR, "UsbEnumeratePort: 2.0 device Recovery Over Current\n", Port));
+    DEBUG ((DEBUG_ERROR, "UsbEnumeratePort: 2.0 device Recovery Over Current (port %d)\n", Port));
   }
 
   if (USB_BIT_IS_SET (PortState.PortChangeStatus, USB_PORT_STAT_C_ENABLE)) {
@@ -961,7 +961,7 @@ UsbEnumeratePort (
     //   on 2.0 roothub does. When over-current has influence on 1.1 device, the port
     //   would be disabled, so it's also necessary to detach and enumerate again.
     //
-    DEBUG ((DEBUG_ERROR, "UsbEnumeratePort: 1.1 device Recovery Over Current\n", Port));
+    DEBUG ((DEBUG_ERROR, "UsbEnumeratePort: 1.1 device Recovery Over Current (port %d)\n", Port));
   }
 
   if (USB_BIT_IS_SET (PortState.PortChangeStatus, USB_PORT_STAT_C_CONNECTION)) {
@@ -969,7 +969,7 @@ UsbEnumeratePort (
     // Case4:
     //   Device connected or disconnected normally.
     //
-    DEBUG ((DEBUG_INFO, "UsbEnumeratePort: Device Connect/Disconnect Normally\n", Port));
+    DEBUG ((DEBUG_INFO, "UsbEnumeratePort: Device Connect/Disconnect Normally (port %d)\n", Port));
   }
 
   //
diff --git a/MdeModulePkg/Core/Dxe/Image/Image.c b/MdeModulePkg/Core/Dxe/Image/Image.c
index 68bde5c15c52..06cc6744b8c6 100644
--- a/MdeModulePkg/Core/Dxe/Image/Image.c
+++ b/MdeModulePkg/Core/Dxe/Image/Image.c
@@ -1741,7 +1741,7 @@ CoreStartImage (
   if ((Image->ExitDataSize != 0) || (Image->ExitData != NULL)) {
     DEBUG ((DEBUG_LOAD, "StartImage: ExitDataSize %d, ExitData %p", (UINT32)Image->ExitDataSize, Image->ExitData));
     if (Image->ExitData != NULL) {
-      DEBUG ((DEBUG_LOAD, " (%hs)", Image->ExitData));
+      DEBUG ((DEBUG_LOAD, " (%s)", Image->ExitData));
     }
 
     DEBUG ((DEBUG_LOAD, "\n"));
diff --git a/MdeModulePkg/Library/DisplayUpdateProgressLibGraphics/DisplayUpdateProgressLibGraphics.c b/MdeModulePkg/Library/DisplayUpdateProgressLibGraphics/DisplayUpdateProgressLibGraphics.c
index 83053464e06e..6b012fed35db 100644
--- a/MdeModulePkg/Library/DisplayUpdateProgressLibGraphics/DisplayUpdateProgressLibGraphics.c
+++ b/MdeModulePkg/Library/DisplayUpdateProgressLibGraphics/DisplayUpdateProgressLibGraphics.c
@@ -148,7 +148,7 @@ FindDim (
                   (VOID **)&BootLogo
                   );
   if ((BootLogo == NULL) || (EFI_ERROR (Status))) {
-    DEBUG ((DEBUG_ERROR, "Failed to locate gEdkiiBootLogo2ProtocolGuid.  No Progress bar support. \n", Status));
+    DEBUG ((DEBUG_ERROR, "Failed to locate gEdkiiBootLogo2ProtocolGuid Status = %r.  No Progress bar support. \n", Status));
     return;
   }
 
-- 
2.28.0.windows.1

[PATCH v1 4/5] RedfishPkg/RedfishRestExDxe: Remove extra debug macro argument

[Michael Kubacki]

From: Michael Kubacki <michael.kubacki@microsoft.com>

The debug macro argument in this change is removed since it does
have a corresponding print specifier in the debug message string.

Cc: Abner Chang <abner.chang@amd.com>
Cc: Nickle Wang <nickle.wang@hpe.com>
Signed-off-by: Michael Kubacki <michael.kubacki@microsoft.com>
---
 RedfishPkg/RedfishRestExDxe/RedfishRestExProtocol.c | 2 +-
 1 file changed, 1 insertion(+), 1 deletion(-)

diff --git a/RedfishPkg/RedfishRestExDxe/RedfishRestExProtocol.c b/RedfishPkg/RedfishRestExDxe/RedfishRestExProtocol.c
index f224104ad673..4b61fc01adc4 100644
--- a/RedfishPkg/RedfishRestExDxe/RedfishRestExProtocol.c
+++ b/RedfishPkg/RedfishRestExDxe/RedfishRestExProtocol.c
@@ -224,7 +224,7 @@ ReSendRequest:;
     DEBUG ((DEBUG_INFO, "HTTP_STATUS_200_OK\n"));
 
     if (SendChunkProcess == HttpIoSendChunkHeaderZeroContent) {
-      DEBUG ((DEBUG_INFO, "This is chunk transfer, start to send all chunks.", ResponseData->Response.StatusCode));
+      DEBUG ((DEBUG_INFO, "This is chunk transfer, start to send all chunks."));
       SendChunkProcess++;
       goto ReSendRequest;
     }
-- 
2.28.0.windows.1

[PATCH v1 5/5] SecurityPkg/SmmTcg2PhysicalPresenceLib: Add missing debug print specifier

[Michael Kubacki]

From: Michael Kubacki <michael.kubacki@microsoft.com>

The debug macro modified in this change was missing a print specifier
for a debug message argument given.

Cc: Jiewen Yao <jiewen.yao@intel.com>
Cc: Jian J Wang <jian.j.wang@intel.com>
Cc: Qi Zhang <qi1.zhang@intel.com>
Cc: Rahul Kumar <rahul1.kumar@intel.com>
Signed-off-by: Michael Kubacki <michael.kubacki@microsoft.com>
---
 SecurityPkg/Library/SmmTcg2PhysicalPresenceLib/MmTcg2PhysicalPresenceLibCommon.c | 2 +-
 1 file changed, 1 insertion(+), 1 deletion(-)

diff --git a/SecurityPkg/Library/SmmTcg2PhysicalPresenceLib/MmTcg2PhysicalPresenceLibCommon.c b/SecurityPkg/Library/SmmTcg2PhysicalPresenceLib/MmTcg2PhysicalPresenceLibCommon.c
index 1fbfc00547cd..f2ab4f125007 100644
--- a/SecurityPkg/Library/SmmTcg2PhysicalPresenceLib/MmTcg2PhysicalPresenceLibCommon.c
+++ b/SecurityPkg/Library/SmmTcg2PhysicalPresenceLib/MmTcg2PhysicalPresenceLibCommon.c
@@ -176,7 +176,7 @@ Tcg2PhysicalPresenceLibSubmitRequestToPreOSFunctionEx (
   // Sync PPRQ/PPRM from PP Variable if PP submission fails
   //
   if (ReturnCode != TCG_PP_SUBMIT_REQUEST_TO_PREOS_SUCCESS) {
-    DEBUG ((DEBUG_ERROR, "[TPM2] Submit PP Request failure! Sync PPRQ/PPRM with PP variable.\n", Status));
+    DEBUG ((DEBUG_ERROR, "[TPM2] Submit PP Request failure! Sync PPRQ/PPRM with PP variable. Status = %r\n", Status));
     DataSize = sizeof (EFI_TCG2_PHYSICAL_PRESENCE);
     ZeroMem (&PpData, DataSize);
     Status = mTcg2PpSmmVariable->SmmGetVariable (
-- 
2.28.0.windows.1

Re: [PATCH v1 5/5] SecurityPkg/SmmTcg2PhysicalPresenceLib: Add missing debug print specifier

[Yao, Jiewen]

Reviewed-by: Jiewen Yao <Jiewen.yao@intel.com>

> -----Original Message-----
> From: mikuback@linux.microsoft.com <mikuback@linux.microsoft.com>
> Sent: Wednesday, August 3, 2022 2:04 AM
> To: devel@edk2.groups.io
> Cc: Yao, Jiewen <jiewen.yao@intel.com>; Wang, Jian J <jian.j.wang@intel.com>;
> Zhang, Qi1 <qi1.zhang@intel.com>; Kumar, Rahul R <rahul.r.kumar@intel.com>
> Subject: [PATCH v1 5/5] SecurityPkg/SmmTcg2PhysicalPresenceLib: Add missing
> debug print specifier
> 
> From: Michael Kubacki <michael.kubacki@microsoft.com>
> 
> The debug macro modified in this change was missing a print specifier
> for a debug message argument given.
> 
> Cc: Jiewen Yao <jiewen.yao@intel.com>
> Cc: Jian J Wang <jian.j.wang@intel.com>
> Cc: Qi Zhang <qi1.zhang@intel.com>
> Cc: Rahul Kumar <rahul1.kumar@intel.com>
> Signed-off-by: Michael Kubacki <michael.kubacki@microsoft.com>
> ---
> 
> SecurityPkg/Library/SmmTcg2PhysicalPresenceLib/MmTcg2PhysicalPresenceLib
> Common.c | 2 +-
>  1 file changed, 1 insertion(+), 1 deletion(-)
> 
> diff --git
> a/SecurityPkg/Library/SmmTcg2PhysicalPresenceLib/MmTcg2PhysicalPresenceLi
> bCommon.c
> b/SecurityPkg/Library/SmmTcg2PhysicalPresenceLib/MmTcg2PhysicalPresenceLi
> bCommon.c
> index 1fbfc00547cd..f2ab4f125007 100644
> ---
> a/SecurityPkg/Library/SmmTcg2PhysicalPresenceLib/MmTcg2PhysicalPresenceLi
> bCommon.c
> +++
> b/SecurityPkg/Library/SmmTcg2PhysicalPresenceLib/MmTcg2PhysicalPresenceLi
> bCommon.c
> @@ -176,7 +176,7 @@
> Tcg2PhysicalPresenceLibSubmitRequestToPreOSFunctionEx (
>    // Sync PPRQ/PPRM from PP Variable if PP submission fails
>    //
>    if (ReturnCode != TCG_PP_SUBMIT_REQUEST_TO_PREOS_SUCCESS) {
> -    DEBUG ((DEBUG_ERROR, "[TPM2] Submit PP Request failure! Sync
> PPRQ/PPRM with PP variable.\n", Status));
> +    DEBUG ((DEBUG_ERROR, "[TPM2] Submit PP Request failure! Sync
> PPRQ/PPRM with PP variable. Status = %r\n", Status));
>      DataSize = sizeof (EFI_TCG2_PHYSICAL_PRESENCE);
>      ZeroMem (&PpData, DataSize);
>      Status = mTcg2PpSmmVariable->SmmGetVariable (
> --
> 2.28.0.windows.1

Re: [PATCH v1 4/5] RedfishPkg/RedfishRestExDxe: Remove extra debug macro argument

[Chang, Abner]

[AMD Official Use Only - General]

Reviewed-by: Abner Chang <abner.chang@amd.com>

> -----Original Message-----
> From: mikuback@linux.microsoft.com <mikuback@linux.microsoft.com>
> Sent: Wednesday, August 3, 2022 2:04 AM
> To: devel@edk2.groups.io
> Cc: Chang, Abner <Abner.Chang@amd.com>; Nickle Wang
> <nickle.wang@hpe.com>
> Subject: [PATCH v1 4/5] RedfishPkg/RedfishRestExDxe: Remove extra debug
> macro argument
> 
> [CAUTION: External Email]
> 
> From: Michael Kubacki <michael.kubacki@microsoft.com>
> 
> The debug macro argument in this change is removed since it does have a
> corresponding print specifier in the debug message string.
> 
> Cc: Abner Chang <abner.chang@amd.com>
> Cc: Nickle Wang <nickle.wang@hpe.com>
> Signed-off-by: Michael Kubacki <michael.kubacki@microsoft.com>
> ---
>  RedfishPkg/RedfishRestExDxe/RedfishRestExProtocol.c | 2 +-
>  1 file changed, 1 insertion(+), 1 deletion(-)
> 
> diff --git a/RedfishPkg/RedfishRestExDxe/RedfishRestExProtocol.c
> b/RedfishPkg/RedfishRestExDxe/RedfishRestExProtocol.c
> index f224104ad673..4b61fc01adc4 100644
> --- a/RedfishPkg/RedfishRestExDxe/RedfishRestExProtocol.c
> +++ b/RedfishPkg/RedfishRestExDxe/RedfishRestExProtocol.c
> @@ -224,7 +224,7 @@ ReSendRequest:;
>      DEBUG ((DEBUG_INFO, "HTTP_STATUS_200_OK\n"));
> 
>      if (SendChunkProcess == HttpIoSendChunkHeaderZeroContent) {
> -      DEBUG ((DEBUG_INFO, "This is chunk transfer, start to send all chunks.",
> ResponseData->Response.StatusCode));
> +      DEBUG ((DEBUG_INFO, "This is chunk transfer, start to send all
> + chunks."));
>        SendChunkProcess++;
>        goto ReSendRequest;
>      }
> --
> 2.28.0.windows.1

Re: [PATCH v1 3/5] MdeModulePkg: Fix imbalanced debug macros

[Wu, Hao A]

For changes in:
MdeModulePkg/Bus/Pci/SdMmcPciHcDxe/SdMmcPciHci.c
MdeModulePkg/Bus/Usb/UsbBusDxe/UsbEnumer.c

Reviewed-by: Hao A Wu <hao.a.wu@intel.com>

Best Regards,
Hao Wu

> -----Original Message-----
> From: mikuback@linux.microsoft.com <mikuback@linux.microsoft.com>
> Sent: Wednesday, August 3, 2022 2:04 AM
> To: devel@edk2.groups.io
> Cc: Bi, Dandan <dandan.bi@intel.com>; Jiang, Guomin
> <guomin.jiang@intel.com>; Wu, Hao A <hao.a.wu@intel.com>; Wang, Jian J
> <jian.j.wang@intel.com>; Gao, Liming <gaoliming@byosoft.com.cn>; Ni, Ray
> <ray.ni@intel.com>
> Subject: [PATCH v1 3/5] MdeModulePkg: Fix imbalanced debug macros
> 
> From: Michael Kubacki <michael.kubacki@microsoft.com>
> 
> Updates debug macros in the package that have an imbalanced number of
> print specifiers to arguments. These changes try to preserve what was likely
> intended by the author. In cases information was missing due to the bug, the
> specifier may be removed since it was not previously accurately printing the
> expected value.
> 
> Cc: Dandan Bi <dandan.bi@intel.com>
> Cc: Guomin Jiang <guomin.jiang@intel.com>
> Cc: Hao A Wu <hao.a.wu@intel.com>
> Cc: Jian J Wang <jian.j.wang@intel.com>
> Cc: Liming Gao <gaoliming@byosoft.com.cn>
> Cc: Liming Gao <gaoliming@byosoft.com.cn>
> Cc: Ray Ni <ray.ni@intel.com>
> Signed-off-by: Michael Kubacki <michael.kubacki@microsoft.com>
> ---
>  MdeModulePkg/Bus/Pci/SdMmcPciHcDxe/SdMmcPciHci.c
> | 2 +-
>  MdeModulePkg/Bus/Usb/UsbBusDxe/UsbEnumer.c
> | 8 ++++----
>  MdeModulePkg/Core/Dxe/Image/Image.c                                                      | 2 +-
> 
> MdeModulePkg/Library/DisplayUpdateProgressLibGraphics/DisplayUpdateP
> rogressLibGraphics.c | 2 +-
>  4 files changed, 7 insertions(+), 7 deletions(-)
> 
> diff --git a/MdeModulePkg/Bus/Pci/SdMmcPciHcDxe/SdMmcPciHci.c
> b/MdeModulePkg/Bus/Pci/SdMmcPciHcDxe/SdMmcPciHci.c
> index 53b63ab52b93..dd45167a009e 100644
> --- a/MdeModulePkg/Bus/Pci/SdMmcPciHcDxe/SdMmcPciHci.c
> +++ b/MdeModulePkg/Bus/Pci/SdMmcPciHcDxe/SdMmcPciHci.c
> @@ -64,7 +64,7 @@ DumpCapabilityReg (
>    DEBUG ((DEBUG_INFO, "   Driver Type D     %a\n", Capability->DriverTypeD ?
> "TRUE" : "FALSE"));
>    DEBUG ((DEBUG_INFO, "   Driver Type 4     %a\n", Capability->DriverType4 ?
> "TRUE" : "FALSE"));
>    if (Capability->TimerCount == 0) {
> -    DEBUG ((DEBUG_INFO, "   Retuning TimerCnt Disabled\n", 2 * (Capability-
> >TimerCount - 1)));
> +    DEBUG ((DEBUG_INFO, "   Retuning TimerCnt Disabled\n"));
>    } else {
>      DEBUG ((DEBUG_INFO, "   Retuning TimerCnt %dseconds\n", 2 *
> (Capability->TimerCount - 1)));
>    }
> diff --git a/MdeModulePkg/Bus/Usb/UsbBusDxe/UsbEnumer.c
> b/MdeModulePkg/Bus/Usb/UsbBusDxe/UsbEnumer.c
> index 5495b324b381..aed34596f469 100644
> --- a/MdeModulePkg/Bus/Usb/UsbBusDxe/UsbEnumer.c
> +++ b/MdeModulePkg/Bus/Usb/UsbBusDxe/UsbEnumer.c
> @@ -941,7 +941,7 @@ UsbEnumeratePort (
>        //   which probably is caused by short circuit. It has to wait system
> hardware
>        //   to perform recovery.
>        //
> -      DEBUG ((DEBUG_ERROR, "UsbEnumeratePort: Critical Over Current\n",
> Port));
> +      DEBUG ((DEBUG_ERROR, "UsbEnumeratePort: Critical Over Current
> + (port %d)\n", Port));
>        return EFI_DEVICE_ERROR;
>      }
> 
> @@ -951,7 +951,7 @@ UsbEnumeratePort (
>      //   over current. As a result, all ports are nearly power-off, so
>      //   it's necessary to detach and enumerate all ports again.
>      //
> -    DEBUG ((DEBUG_ERROR, "UsbEnumeratePort: 2.0 device Recovery Over
> Current\n", Port));
> +    DEBUG ((DEBUG_ERROR, "UsbEnumeratePort: 2.0 device Recovery Over
> + Current (port %d)\n", Port));
>    }
> 
>    if (USB_BIT_IS_SET (PortState.PortChangeStatus,
> USB_PORT_STAT_C_ENABLE)) { @@ -961,7 +961,7 @@ UsbEnumeratePort (
>      //   on 2.0 roothub does. When over-current has influence on 1.1 device,
> the port
>      //   would be disabled, so it's also necessary to detach and enumerate
> again.
>      //
> -    DEBUG ((DEBUG_ERROR, "UsbEnumeratePort: 1.1 device Recovery Over
> Current\n", Port));
> +    DEBUG ((DEBUG_ERROR, "UsbEnumeratePort: 1.1 device Recovery Over
> + Current (port %d)\n", Port));
>    }
> 
>    if (USB_BIT_IS_SET (PortState.PortChangeStatus,
> USB_PORT_STAT_C_CONNECTION)) { @@ -969,7 +969,7 @@
> UsbEnumeratePort (
>      // Case4:
>      //   Device connected or disconnected normally.
>      //
> -    DEBUG ((DEBUG_INFO, "UsbEnumeratePort: Device Connect/Disconnect
> Normally\n", Port));
> +    DEBUG ((DEBUG_INFO, "UsbEnumeratePort: Device Connect/Disconnect
> + Normally (port %d)\n", Port));
>    }
> 
>    //
> diff --git a/MdeModulePkg/Core/Dxe/Image/Image.c
> b/MdeModulePkg/Core/Dxe/Image/Image.c
> index 68bde5c15c52..06cc6744b8c6 100644
> --- a/MdeModulePkg/Core/Dxe/Image/Image.c
> +++ b/MdeModulePkg/Core/Dxe/Image/Image.c
> @@ -1741,7 +1741,7 @@ CoreStartImage (
>    if ((Image->ExitDataSize != 0) || (Image->ExitData != NULL)) {
>      DEBUG ((DEBUG_LOAD, "StartImage: ExitDataSize %d, ExitData %p",
> (UINT32)Image->ExitDataSize, Image->ExitData));
>      if (Image->ExitData != NULL) {
> -      DEBUG ((DEBUG_LOAD, " (%hs)", Image->ExitData));
> +      DEBUG ((DEBUG_LOAD, " (%s)", Image->ExitData));
>      }
> 
>      DEBUG ((DEBUG_LOAD, "\n"));
> diff --git
> a/MdeModulePkg/Library/DisplayUpdateProgressLibGraphics/DisplayUpdat
> eProgressLibGraphics.c
> b/MdeModulePkg/Library/DisplayUpdateProgressLibGraphics/DisplayUpdat
> eProgressLibGraphics.c
> index 83053464e06e..6b012fed35db 100644
> ---
> a/MdeModulePkg/Library/DisplayUpdateProgressLibGraphics/DisplayUpdat
> eProgressLibGraphics.c
> +++
> b/MdeModulePkg/Library/DisplayUpdateProgressLibGraphics/DisplayUpdat
> +++ eProgressLibGraphics.c
> @@ -148,7 +148,7 @@ FindDim (
>                    (VOID **)&BootLogo
>                    );
>    if ((BootLogo == NULL) || (EFI_ERROR (Status))) {
> -    DEBUG ((DEBUG_ERROR, "Failed to locate gEdkiiBootLogo2ProtocolGuid.
> No Progress bar support. \n", Status));
> +    DEBUG ((DEBUG_ERROR, "Failed to locate gEdkiiBootLogo2ProtocolGuid
> + Status = %r.  No Progress bar support. \n", Status));
>      return;
>    }
> 
> --
> 2.28.0.windows.1

Re: [PATCH v1 1/5] ArmPlatformPkg/NorFlashDxe: Remove unused debug print specifier

[Ard Biesheuvel]

On Tue, 2 Aug 2022 at 20:04, <mikuback@linux.microsoft.com> wrote:
>
> From: Michael Kubacki <michael.kubacki@microsoft.com>
>
> These debug messages are repeated in both NorFlashBlockIoReadBlocks()
> and NorFlashBlockIoWriteBlocks():
>
>   "NorFlashBlockIoWriteBlocks(MediaId=0x%x, Lba=%ld, BufferSize=0x%x"
>   "bytes (%d kB), BufferPtr @ 0x%08x)\n"
>
> Although this requires 5 arguments, only 4 are provided. The kilobyte
> value was never given.
>
> This change removes that specifier so the 4 arguments match the 4
> specifiers in the debug macro.
>
> Cc: Leif Lindholm <quic_llindhol@quicinc.com>
> Cc: Ard Biesheuvel <ardb+tianocore@kernel.org>
> Signed-off-by: Michael Kubacki <michael.kubacki@microsoft.com>

Reviewed-by: Ard Biesheuve; <ardb@kernel.org>

> ---
>  ArmPlatformPkg/Drivers/NorFlashDxe/NorFlashBlockIoDxe.c | 4 ++--
>  1 file changed, 2 insertions(+), 2 deletions(-)
>
> diff --git a/ArmPlatformPkg/Drivers/NorFlashDxe/NorFlashBlockIoDxe.c b/ArmPlatformPkg/Drivers/NorFlashDxe/NorFlashBlockIoDxe.c
> index 5afab0a79fa2..e671108e2bcf 100644
> --- a/ArmPlatformPkg/Drivers/NorFlashDxe/NorFlashBlockIoDxe.c
> +++ b/ArmPlatformPkg/Drivers/NorFlashDxe/NorFlashBlockIoDxe.c
> @@ -54,7 +54,7 @@ NorFlashBlockIoReadBlocks (
>    Instance = INSTANCE_FROM_BLKIO_THIS (This);
>    Media    = This->Media;
>
> -  DEBUG ((DEBUG_BLKIO, "NorFlashBlockIoReadBlocks(MediaId=0x%x, Lba=%ld, BufferSize=0x%x bytes (%d kB), BufferPtr @ 0x%08x)\n", MediaId, Lba, BufferSizeInBytes, Buffer));
> +  DEBUG ((DEBUG_BLKIO, "NorFlashBlockIoReadBlocks(MediaId=0x%x, Lba=%ld, BufferSize=0x%x bytes, BufferPtr @ 0x%08x)\n", MediaId, Lba, BufferSizeInBytes, Buffer));
>
>    if (!Media) {
>      Status = EFI_INVALID_PARAMETER;
> @@ -89,7 +89,7 @@ NorFlashBlockIoWriteBlocks (
>
>    Instance = INSTANCE_FROM_BLKIO_THIS (This);
>
> -  DEBUG ((DEBUG_BLKIO, "NorFlashBlockIoWriteBlocks(MediaId=0x%x, Lba=%ld, BufferSize=0x%x bytes (%d kB), BufferPtr @ 0x%08x)\n", MediaId, Lba, BufferSizeInBytes, Buffer));
> +  DEBUG ((DEBUG_BLKIO, "NorFlashBlockIoWriteBlocks(MediaId=0x%x, Lba=%ld, BufferSize=0x%x bytes, BufferPtr @ 0x%08x)\n", MediaId, Lba, BufferSizeInBytes, Buffer));
>
>    if ( !This->Media->MediaPresent ) {
>      Status = EFI_NO_MEDIA;
> --
> 2.28.0.windows.1
>

回复: [PATCH v1 3/5] MdeModulePkg: Fix imbalanced debug macros

[gaoliming]

Reviewed-by: Liming Gao <gaoliming@byosoft.com.cn>

> -----邮件原件-----
> 发件人: mikuback@linux.microsoft.com <mikuback@linux.microsoft.com>
> 发送时间: 2022年8月3日 2:04
> 收件人: devel@edk2.groups.io
> 抄送: Dandan Bi <dandan.bi@intel.com>; Guomin Jiang
> <guomin.jiang@intel.com>; Hao A Wu <hao.a.wu@intel.com>; Jian J Wang
> <jian.j.wang@intel.com>; Liming Gao <gaoliming@byosoft.com.cn>; Ray Ni
> <ray.ni@intel.com>
> 主题: [PATCH v1 3/5] MdeModulePkg: Fix imbalanced debug macros
> 
> From: Michael Kubacki <michael.kubacki@microsoft.com>
> 
> Updates debug macros in the package that have an imbalanced number
> of print specifiers to arguments. These changes try to preserve
> what was likely intended by the author. In cases information was
> missing due to the bug, the specifier may be removed since it was
> not previously accurately printing the expected value.
> 
> Cc: Dandan Bi <dandan.bi@intel.com>
> Cc: Guomin Jiang <guomin.jiang@intel.com>
> Cc: Hao A Wu <hao.a.wu@intel.com>
> Cc: Jian J Wang <jian.j.wang@intel.com>
> Cc: Liming Gao <gaoliming@byosoft.com.cn>
> Cc: Liming Gao <gaoliming@byosoft.com.cn>
> Cc: Ray Ni <ray.ni@intel.com>
> Signed-off-by: Michael Kubacki <michael.kubacki@microsoft.com>
> ---
>  MdeModulePkg/Bus/Pci/SdMmcPciHcDxe/SdMmcPciHci.c
> | 2 +-
>  MdeModulePkg/Bus/Usb/UsbBusDxe/UsbEnumer.c
> | 8 ++++----
>  MdeModulePkg/Core/Dxe/Image/Image.c
> | 2 +-
> 
> MdeModulePkg/Library/DisplayUpdateProgressLibGraphics/DisplayUpdatePr
> ogressLibGraphics.c | 2 +-
>  4 files changed, 7 insertions(+), 7 deletions(-)
> 
> diff --git a/MdeModulePkg/Bus/Pci/SdMmcPciHcDxe/SdMmcPciHci.c
> b/MdeModulePkg/Bus/Pci/SdMmcPciHcDxe/SdMmcPciHci.c
> index 53b63ab52b93..dd45167a009e 100644
> --- a/MdeModulePkg/Bus/Pci/SdMmcPciHcDxe/SdMmcPciHci.c
> +++ b/MdeModulePkg/Bus/Pci/SdMmcPciHcDxe/SdMmcPciHci.c
> @@ -64,7 +64,7 @@ DumpCapabilityReg (
>    DEBUG ((DEBUG_INFO, "   Driver Type D     %a\n",
> Capability->DriverTypeD ? "TRUE" : "FALSE"));
>    DEBUG ((DEBUG_INFO, "   Driver Type 4     %a\n",
> Capability->DriverType4 ? "TRUE" : "FALSE"));
>    if (Capability->TimerCount == 0) {
> -    DEBUG ((DEBUG_INFO, "   Retuning TimerCnt Disabled\n", 2 *
> (Capability->TimerCount - 1)));
> +    DEBUG ((DEBUG_INFO, "   Retuning TimerCnt Disabled\n"));
>    } else {
>      DEBUG ((DEBUG_INFO, "   Retuning TimerCnt %dseconds\n", 2 *
> (Capability->TimerCount - 1)));
>    }
> diff --git a/MdeModulePkg/Bus/Usb/UsbBusDxe/UsbEnumer.c
> b/MdeModulePkg/Bus/Usb/UsbBusDxe/UsbEnumer.c
> index 5495b324b381..aed34596f469 100644
> --- a/MdeModulePkg/Bus/Usb/UsbBusDxe/UsbEnumer.c
> +++ b/MdeModulePkg/Bus/Usb/UsbBusDxe/UsbEnumer.c
> @@ -941,7 +941,7 @@ UsbEnumeratePort (
>        //   which probably is caused by short circuit. It has to wait
system
> hardware
>        //   to perform recovery.
>        //
> -      DEBUG ((DEBUG_ERROR, "UsbEnumeratePort: Critical Over
> Current\n", Port));
> +      DEBUG ((DEBUG_ERROR, "UsbEnumeratePort: Critical Over Current
> (port %d)\n", Port));
>        return EFI_DEVICE_ERROR;
>      }
> 
> @@ -951,7 +951,7 @@ UsbEnumeratePort (
>      //   over current. As a result, all ports are nearly power-off, so
>      //   it's necessary to detach and enumerate all ports again.
>      //
> -    DEBUG ((DEBUG_ERROR, "UsbEnumeratePort: 2.0 device Recovery
> Over Current\n", Port));
> +    DEBUG ((DEBUG_ERROR, "UsbEnumeratePort: 2.0 device Recovery
> Over Current (port %d)\n", Port));
>    }
> 
>    if (USB_BIT_IS_SET (PortState.PortChangeStatus,
> USB_PORT_STAT_C_ENABLE)) {
> @@ -961,7 +961,7 @@ UsbEnumeratePort (
>      //   on 2.0 roothub does. When over-current has influence on 1.1
> device, the port
>      //   would be disabled, so it's also necessary to detach and
enumerate
> again.
>      //
> -    DEBUG ((DEBUG_ERROR, "UsbEnumeratePort: 1.1 device Recovery
> Over Current\n", Port));
> +    DEBUG ((DEBUG_ERROR, "UsbEnumeratePort: 1.1 device Recovery
> Over Current (port %d)\n", Port));
>    }
> 
>    if (USB_BIT_IS_SET (PortState.PortChangeStatus,
> USB_PORT_STAT_C_CONNECTION)) {
> @@ -969,7 +969,7 @@ UsbEnumeratePort (
>      // Case4:
>      //   Device connected or disconnected normally.
>      //
> -    DEBUG ((DEBUG_INFO, "UsbEnumeratePort: Device
> Connect/Disconnect Normally\n", Port));
> +    DEBUG ((DEBUG_INFO, "UsbEnumeratePort: Device
> Connect/Disconnect Normally (port %d)\n", Port));
>    }
> 
>    //
> diff --git a/MdeModulePkg/Core/Dxe/Image/Image.c
> b/MdeModulePkg/Core/Dxe/Image/Image.c
> index 68bde5c15c52..06cc6744b8c6 100644
> --- a/MdeModulePkg/Core/Dxe/Image/Image.c
> +++ b/MdeModulePkg/Core/Dxe/Image/Image.c
> @@ -1741,7 +1741,7 @@ CoreStartImage (
>    if ((Image->ExitDataSize != 0) || (Image->ExitData != NULL)) {
>      DEBUG ((DEBUG_LOAD, "StartImage: ExitDataSize %d, ExitData %p",
> (UINT32)Image->ExitDataSize, Image->ExitData));
>      if (Image->ExitData != NULL) {
> -      DEBUG ((DEBUG_LOAD, " (%hs)", Image->ExitData));
> +      DEBUG ((DEBUG_LOAD, " (%s)", Image->ExitData));
>      }
> 
>      DEBUG ((DEBUG_LOAD, "\n"));
> diff --git
> a/MdeModulePkg/Library/DisplayUpdateProgressLibGraphics/DisplayUpdate
> ProgressLibGraphics.c
> b/MdeModulePkg/Library/DisplayUpdateProgressLibGraphics/DisplayUpdate
> ProgressLibGraphics.c
> index 83053464e06e..6b012fed35db 100644
> ---
> a/MdeModulePkg/Library/DisplayUpdateProgressLibGraphics/DisplayUpdate
> ProgressLibGraphics.c
> +++
> b/MdeModulePkg/Library/DisplayUpdateProgressLibGraphics/DisplayUpdate
> ProgressLibGraphics.c
> @@ -148,7 +148,7 @@ FindDim (
>                    (VOID **)&BootLogo
>                    );
>    if ((BootLogo == NULL) || (EFI_ERROR (Status))) {
> -    DEBUG ((DEBUG_ERROR, "Failed to locate gEdkiiBootLogo2ProtocolGuid.
> No Progress bar support. \n", Status));
> +    DEBUG ((DEBUG_ERROR, "Failed to locate
> gEdkiiBootLogo2ProtocolGuid Status = %r.  No Progress bar support. \n",
> Status));
>      return;
>    }
> 
> --
> 2.28.0.windows.1