[PATCH v3 0/8] Fix imbalanced debug macros

[PATCH v3 0/8] 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.

In some cases it is not obvious what was originally intended so I
have attempted to capture what I believe was the original author's
intention.

V3 Changes:
Updated R-b tags.
1. ArmPlatformPkg/Drivers/NorFlashDxe/NorFlashBlockIoDxe.c:
   - Repeated BufferSizeInBytes argument per Leif's feedback
2. MdeModulePkg/Bus/Pci/NvmExpressPei/NvmExpressPei.c:
   - Fixed an additional error on Line 267

V2 Changes:
These additional errors were found while finishing testing of
the script.

1. Add DynamicTablesPkg/AcpiPpttLibArm patch for the change in
   - DynamicTablesPkg/Library/Acpi/Arm/AcpiPpttLibArm/PpttGenerator.c
2. Update MdeModulePkg patch to include the change in
   - MdeModulePkg/Universal/CapsulePei/UefiCapsule.c
3. Add following to the RedfishPkg fix patch:
   - RedfishPkg/RedfishDiscoverDxe/RedfishDiscoverDxe.c
   - RedfishPkg/RedfishHostInterfaceDxe/RedfishHostInterfaceDxe.c
4. Add NetworkPkg patch for the change in
   - NetworkPkg/TcpDxe/SockInterface.c
5. Add OvmfPkg patch for the change in
   - OvmfPkg/Csm/LegacyBootManagerLib/LegacyBm.c

Cc: Abner Chang <abner.chang@amd.com>
Cc: Alexei Fedorov <Alexei.Fedorov@arm.com>
Cc: Ard Biesheuvel <ardb+tianocore@kernel.org>
Cc: Dandan Bi <dandan.bi@intel.com>
Cc: David Woodhouse <dwmw2@infradead.org>
Cc: Gerd Hoffmann <kraxel@redhat.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: Jiaxin Wu <jiaxin.wu@intel.com>
Cc: Jiewen Yao <jiewen.yao@intel.com>
Cc: Jordan Justen <jordan.l.justen@intel.com>
Cc: Leif Lindholm <quic_llindhol@quicinc.com>
Cc: Liming Gao <gaoliming@byosoft.com.cn>
Cc: Maciej Rabeda <maciej.rabeda@linux.intel.com>
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>
Cc: Sami Mujawar <Sami.Mujawar@arm.com>
Cc: Siyuan Fu <siyuan.fu@intel.com>
Signed-off-by: Michael Kubacki <michael.kubacki@microsoft.com>

Michael Kubacki (8):
  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
  DynamicTablesPkg/AcpiPpttLibArm: Fix debug macro arguments
  NetworkPkg/TcpDxe: Fix debug macro arguments
  OvmfPkg/LegacyBootManagerLib: Fix debug macro arguments

 ArmPlatformPkg/Drivers/NorFlashDxe/NorFlashBlockIoDxe.c                                  |  4 ++--
 DynamicTablesPkg/Library/Acpi/Arm/AcpiPpttLibArm/PpttGenerator.c                         |  3 +--
 FatPkg/FatPei/Gpt.c                                                                      |  2 +-
 MdeModulePkg/Bus/Pci/NvmExpressPei/NvmExpressPei.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 +-
 MdeModulePkg/Universal/CapsulePei/UefiCapsule.c                                          |  2 +-
 NetworkPkg/TcpDxe/SockInterface.c                                                        | 12 ++----------
 OvmfPkg/Csm/LegacyBootManagerLib/LegacyBm.c                                              |  6 +++---
 RedfishPkg/RedfishDiscoverDxe/RedfishDiscoverDxe.c                                       |  6 +++---
 RedfishPkg/RedfishHostInterfaceDxe/RedfishHostInterfaceDxe.c                             |  2 +-
 RedfishPkg/RedfishRestExDxe/RedfishRestExProtocol.c                                      |  2 +-
 SecurityPkg/Library/SmmTcg2PhysicalPresenceLib/MmTcg2PhysicalPresenceLibCommon.c         |  2 +-
 15 files changed, 24 insertions(+), 33 deletions(-)

-- 
2.28.0.windows.1

[PATCH v3 1/8] 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>
Reviewed-by: Ard Biesheuvel <ardb@kernel.org>
Reviewed-by: Leif Lindholm <quic_llindhol@quicinc.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..9d4732c6905a 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 (%d kB), BufferPtr @ 0x%08x)\n", MediaId, Lba, BufferSizeInBytes, 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 v3 2/8] 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 v3 3/8] 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>
Reviewed-by: Hao A Wu <hao.a.wu@intel.com>
Reviewed-by: Liming Gao <gaoliming@byosoft.com.cn>
---
 MdeModulePkg/Bus/Pci/NvmExpressPei/NvmExpressPei.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 +-
 MdeModulePkg/Universal/CapsulePei/UefiCapsule.c                                          | 2 +-
 6 files changed, 9 insertions(+), 9 deletions(-)

diff --git a/MdeModulePkg/Bus/Pci/NvmExpressPei/NvmExpressPei.c b/MdeModulePkg/Bus/Pci/NvmExpressPei/NvmExpressPei.c
index 0d02c2785da7..3e6f75345d98 100644
--- a/MdeModulePkg/Bus/Pci/NvmExpressPei/NvmExpressPei.c
+++ b/MdeModulePkg/Bus/Pci/NvmExpressPei/NvmExpressPei.c
@@ -264,7 +264,7 @@ NvmeInitPrivateData (
   if (EFI_ERROR (Status)) {
     DEBUG ((
       DEBUG_ERROR,
-      "%a: The device path is invalid for Controller %d.\n",
+      "%a: The device path is invalid.\n",
       __FUNCTION__
       ));
     return Status;
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;
   }
 
diff --git a/MdeModulePkg/Universal/CapsulePei/UefiCapsule.c b/MdeModulePkg/Universal/CapsulePei/UefiCapsule.c
index ef60d4e1b7b7..cdeffa911311 100644
--- a/MdeModulePkg/Universal/CapsulePei/UefiCapsule.c
+++ b/MdeModulePkg/Universal/CapsulePei/UefiCapsule.c
@@ -976,7 +976,7 @@ GetScatterGatherHeadEntries (
 
     if (EFI_ERROR (Status)) {
       if (Status != EFI_NOT_FOUND) {
-        DEBUG ((DEBUG_ERROR, "Unexpected error getting Capsule Update variable.  Status = %r\n"));
+        DEBUG ((DEBUG_ERROR, "Unexpected error getting Capsule Update variable.  Status = %r\n", Status));
       }
 
       break;
-- 
2.28.0.windows.1

[PATCH v3 4/8] 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>
Reviewed-by: Abner Chang <abner.chang@amd.com>
---
 RedfishPkg/RedfishDiscoverDxe/RedfishDiscoverDxe.c           | 6 +++---
 RedfishPkg/RedfishHostInterfaceDxe/RedfishHostInterfaceDxe.c | 2 +-
 RedfishPkg/RedfishRestExDxe/RedfishRestExProtocol.c          | 2 +-
 3 files changed, 5 insertions(+), 5 deletions(-)

diff --git a/RedfishPkg/RedfishDiscoverDxe/RedfishDiscoverDxe.c b/RedfishPkg/RedfishDiscoverDxe/RedfishDiscoverDxe.c
index bf50c78c9280..586ecfadc715 100644
--- a/RedfishPkg/RedfishDiscoverDxe/RedfishDiscoverDxe.c
+++ b/RedfishPkg/RedfishDiscoverDxe/RedfishDiscoverDxe.c
@@ -264,12 +264,12 @@ Tcp6GetSubnetInfo (
 
   Status = Tcp6->GetModeData (Tcp6, NULL, NULL, &IpModedata, NULL, NULL);
   if (EFI_ERROR (Status)) {
-    DEBUG ((DEBUG_ERROR, "%a: Can't get IP mode data information\n"));
+    DEBUG ((DEBUG_ERROR, "%a: Can't get IP mode data information\n", __FUNCTION__));
     return Status;
   }
 
   if (IpModedata.AddressCount == 0) {
-    DEBUG ((DEBUG_INFO, "%a: No IPv6 address configured.\n"));
+    DEBUG ((DEBUG_INFO, "%a: No IPv6 address configured.\n", __FUNCTION__));
   }
 
   if (Instance->SubnetAddrInfoIPv6 != NULL) {
@@ -278,7 +278,7 @@ Tcp6GetSubnetInfo (
 
   Instance->SubnetAddrInfoIPv6 = AllocateZeroPool (IpModedata.AddressCount * sizeof (EFI_IP6_ADDRESS_INFO));
   if (Instance->SubnetAddrInfoIPv6 == NULL) {
-    DEBUG ((DEBUG_ERROR, "%a: Failed to allocate memory fir IPv6 subnet address information\n"));
+    DEBUG ((DEBUG_ERROR, "%a: Failed to allocate memory for IPv6 subnet address information\n", __FUNCTION__));
     return EFI_OUT_OF_RESOURCES;
   }
 
diff --git a/RedfishPkg/RedfishHostInterfaceDxe/RedfishHostInterfaceDxe.c b/RedfishPkg/RedfishHostInterfaceDxe/RedfishHostInterfaceDxe.c
index 8a05764ac605..623350bc261c 100644
--- a/RedfishPkg/RedfishHostInterfaceDxe/RedfishHostInterfaceDxe.c
+++ b/RedfishPkg/RedfishHostInterfaceDxe/RedfishHostInterfaceDxe.c
@@ -119,7 +119,7 @@ RedfishCreateSmbiosTable42 (
     } else {
       NewProtocolRecords = ReallocatePool (CurrentProtocolsDataLength, NewProtocolsDataLength, (VOID *)ProtocolRecords);
       if (NewProtocolRecords == NULL) {
-        DEBUG ((DEBUG_ERROR, "%a: Fail to allocate memory for Redfish host interface protocol data."));
+        DEBUG ((DEBUG_ERROR, "%a: Fail to allocate memory for Redfish host interface protocol data.", __FUNCTION__));
         FreePool (ProtocolRecords);
         FreePool (ProtocolRecord);
         return EFI_OUT_OF_RESOURCES;
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 v3 5/8] 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>
Reviewed-by: Jiewen Yao <Jiewen.yao@intel.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

[PATCH v3 6/8] DynamicTablesPkg/AcpiPpttLibArm: Fix debug macro arguments

[Michael Kubacki]

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

Cc: Sami Mujawar <Sami.Mujawar@arm.com>
Cc: Alexei Fedorov <Alexei.Fedorov@arm.com>
Signed-off-by: Michael Kubacki <michael.kubacki@microsoft.com>
Reviewed-by: Sami Mujawar <sami.mujawar@arm.com>
---
 DynamicTablesPkg/Library/Acpi/Arm/AcpiPpttLibArm/PpttGenerator.c | 3 +--
 1 file changed, 1 insertion(+), 2 deletions(-)

diff --git a/DynamicTablesPkg/Library/Acpi/Arm/AcpiPpttLibArm/PpttGenerator.c b/DynamicTablesPkg/Library/Acpi/Arm/AcpiPpttLibArm/PpttGenerator.c
index 59001378c4e0..78fa63ff47e5 100644
--- a/DynamicTablesPkg/Library/Acpi/Arm/AcpiPpttLibArm/PpttGenerator.c
+++ b/DynamicTablesPkg/Library/Acpi/Arm/AcpiPpttLibArm/PpttGenerator.c
@@ -436,7 +436,6 @@ IsGicCTokenEqual (
       "the same GICC Info object. ACPI Processor IDs are not unique. " \
       "GicCToken = %p.\n",
       Index1,
-      IndexedObject1->Token,
       Index2,
       ProcNode1->GicCToken
       ));
@@ -566,7 +565,7 @@ AddProcHierarchyNodes (
         DEBUG ((
           DEBUG_ERROR,
           "ERROR: PPTT: Failed to get parent processor hierarchy node " \
-          "reference. Token = %p, Status = %r\n",
+          "reference. ParentToken = %p. ChildToken = %p. Status = %r\n",
           ProcInfoNode->ParentToken,
           ProcInfoNode->Token,
           Status
-- 
2.28.0.windows.1

[PATCH v3 7/8] NetworkPkg/TcpDxe: Fix debug macro arguments

[Michael Kubacki]

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

Removes Status argument that is not needed from DEBUG macros.

Cc: Maciej Rabeda <maciej.rabeda@linux.intel.com>
Cc: Jiaxin Wu <jiaxin.wu@intel.com>
Cc: Siyuan Fu <siyuan.fu@intel.com>
Signed-off-by: Michael Kubacki <michael.kubacki@microsoft.com>
Reviewed-by: Maciej Rabeda <maciej.rabeda@linux.intel.com>
---
 NetworkPkg/TcpDxe/SockInterface.c | 12 ++----------
 1 file changed, 2 insertions(+), 10 deletions(-)

diff --git a/NetworkPkg/TcpDxe/SockInterface.c b/NetworkPkg/TcpDxe/SockInterface.c
index 413d6e1373f3..6e8cbb74a86b 100644
--- a/NetworkPkg/TcpDxe/SockInterface.c
+++ b/NetworkPkg/TcpDxe/SockInterface.c
@@ -661,11 +661,7 @@ SockSend (
                   );
 
     if (NULL == SockToken) {
-      DEBUG (
-        (DEBUG_ERROR,
-         "SockSend: Failed to buffer IO token into socket processing SndToken List\n",
-         Status)
-        );
+      DEBUG ((DEBUG_ERROR, "SockSend: Failed to buffer IO token into socket processing SndToken List\n"));
 
       Status = EFI_OUT_OF_RESOURCES;
       goto Exit;
@@ -674,11 +670,7 @@ SockSend (
     Status = SockProcessTcpSndData (Sock, TxData);
 
     if (EFI_ERROR (Status)) {
-      DEBUG (
-        (DEBUG_ERROR,
-         "SockSend: Failed to process Snd Data\n",
-         Status)
-        );
+      DEBUG ((DEBUG_ERROR, "SockSend: Failed to process Snd Data\n"));
 
       RemoveEntryList (&(SockToken->TokenList));
       FreePool (SockToken);
-- 
2.28.0.windows.1

[PATCH v3 8/8] OvmfPkg/LegacyBootManagerLib: Fix debug macro arguments

[Michael Kubacki]

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

The DEBUG macro updated in this patch previously contained 11 print
specifiers in the debug string but passeed 13 arguments. This change
attempts to update the macro to the author's intention so the number
of specifiers match the number of arguments.

Cc: Ard Biesheuvel <ardb+tianocore@kernel.org>
Cc: Jiewen Yao <jiewen.yao@intel.com>
Cc: Jordan Justen <jordan.l.justen@intel.com>
Cc: Gerd Hoffmann <kraxel@redhat.com>
Cc: David Woodhouse <dwmw2@infradead.org>
Signed-off-by: Michael Kubacki <michael.kubacki@microsoft.com>
---
 OvmfPkg/Csm/LegacyBootManagerLib/LegacyBm.c | 6 +++---
 1 file changed, 3 insertions(+), 3 deletions(-)

diff --git a/OvmfPkg/Csm/LegacyBootManagerLib/LegacyBm.c b/OvmfPkg/Csm/LegacyBootManagerLib/LegacyBm.c
index 032aacba68de..2e92bce726a9 100644
--- a/OvmfPkg/Csm/LegacyBootManagerLib/LegacyBm.c
+++ b/OvmfPkg/Csm/LegacyBootManagerLib/LegacyBm.c
@@ -1159,8 +1159,8 @@ LegacyBmPrintBbsTable (
   UINT16  Index;
 
   DEBUG ((DEBUG_INFO, "\n"));
-  DEBUG ((DEBUG_INFO, " NO  Prio bb/dd/ff cl/sc Type Stat segm:offs\n"));
-  DEBUG ((DEBUG_INFO, "=============================================\n"));
+  DEBUG ((DEBUG_INFO, " NO  Prio bb/dd/ff cl/sc Type Stat segm:offs mseg dseg\n"));
+  DEBUG ((DEBUG_INFO, "======================================================\n"));
   for (Index = 0; Index < BbsCount; Index++) {
     if (!LegacyBmValidBbsEntry (&LocalBbsTable[Index])) {
       continue;
@@ -1168,7 +1168,7 @@ LegacyBmPrintBbsTable (
 
     DEBUG (
       (DEBUG_INFO,
-       " %02x: %04x %02x/%02x/%02x %02x/%02x %04x %04x %04x:%04x\n",
+       " %02x: %04x %02x/%02x/%02x %02x/%02x %04x %04x %04x:%04x %04x %04x\n",
        (UINTN)Index,
        (UINTN)LocalBbsTable[Index].BootPriority,
        (UINTN)LocalBbsTable[Index].Bus,
-- 
2.28.0.windows.1

Re: [PATCH v3 8/8] OvmfPkg/LegacyBootManagerLib: Fix debug macro arguments

[Gerd Hoffmann]

On Fri, Sep 02, 2022 at 06:22:06PM -0400, mikuback@linux.microsoft.com wrote:
> From: Michael Kubacki <michael.kubacki@microsoft.com>
> 
> The DEBUG macro updated in this patch previously contained 11 print
> specifiers in the debug string but passeed 13 arguments. This change
> attempts to update the macro to the author's intention so the number
> of specifiers match the number of arguments.
> 
> Cc: Ard Biesheuvel <ardb+tianocore@kernel.org>
> Cc: Jiewen Yao <jiewen.yao@intel.com>
> Cc: Jordan Justen <jordan.l.justen@intel.com>
> Cc: Gerd Hoffmann <kraxel@redhat.com>
> Cc: David Woodhouse <dwmw2@infradead.org>
> Signed-off-by: Michael Kubacki <michael.kubacki@microsoft.com>

Acked-by: Gerd Hoffmann <kraxel@redhat.com>

Re: [edk2-devel] [PATCH v3 4/8] RedfishPkg/RedfishRestExDxe: Remove extra debug macro argument

[nickle]

[-- Attachment #1: Type: text/plain, Size: 206 bytes --]

Reviewed-by: Nickle Wang <nickle@csie.io>

Please help to update my email in commit message since it is updated recently.

On Sat, Sep 3, 2022 at 06:22 AM, Michael Kubacki wrote:

> 
> Reviewed-by:

[-- Attachment #2: Type: text/html, Size: 295 bytes --]

Re: [PATCH v3 8/8] OvmfPkg/LegacyBootManagerLib: Fix debug macro arguments

[Ard Biesheuvel]

On Sat, 3 Sept 2022 at 00:23, <mikuback@linux.microsoft.com> wrote:
>
> From: Michael Kubacki <michael.kubacki@microsoft.com>
>
> The DEBUG macro updated in this patch previously contained 11 print
> specifiers in the debug string but passeed 13 arguments. This change
> attempts to update the macro to the author's intention so the number
> of specifiers match the number of arguments.
>
> Cc: Ard Biesheuvel <ardb+tianocore@kernel.org>
> Cc: Jiewen Yao <jiewen.yao@intel.com>
> Cc: Jordan Justen <jordan.l.justen@intel.com>
> Cc: Gerd Hoffmann <kraxel@redhat.com>
> Cc: David Woodhouse <dwmw2@infradead.org>
> Signed-off-by: Michael Kubacki <michael.kubacki@microsoft.com>

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

> ---
>  OvmfPkg/Csm/LegacyBootManagerLib/LegacyBm.c | 6 +++---
>  1 file changed, 3 insertions(+), 3 deletions(-)
>
> diff --git a/OvmfPkg/Csm/LegacyBootManagerLib/LegacyBm.c b/OvmfPkg/Csm/LegacyBootManagerLib/LegacyBm.c
> index 032aacba68de..2e92bce726a9 100644
> --- a/OvmfPkg/Csm/LegacyBootManagerLib/LegacyBm.c
> +++ b/OvmfPkg/Csm/LegacyBootManagerLib/LegacyBm.c
> @@ -1159,8 +1159,8 @@ LegacyBmPrintBbsTable (
>    UINT16  Index;
>
>    DEBUG ((DEBUG_INFO, "\n"));
> -  DEBUG ((DEBUG_INFO, " NO  Prio bb/dd/ff cl/sc Type Stat segm:offs\n"));
> -  DEBUG ((DEBUG_INFO, "=============================================\n"));
> +  DEBUG ((DEBUG_INFO, " NO  Prio bb/dd/ff cl/sc Type Stat segm:offs mseg dseg\n"));
> +  DEBUG ((DEBUG_INFO, "======================================================\n"));
>    for (Index = 0; Index < BbsCount; Index++) {
>      if (!LegacyBmValidBbsEntry (&LocalBbsTable[Index])) {
>        continue;
> @@ -1168,7 +1168,7 @@ LegacyBmPrintBbsTable (
>
>      DEBUG (
>        (DEBUG_INFO,
> -       " %02x: %04x %02x/%02x/%02x %02x/%02x %04x %04x %04x:%04x\n",
> +       " %02x: %04x %02x/%02x/%02x %02x/%02x %04x %04x %04x:%04x %04x %04x\n",
>         (UINTN)Index,
>         (UINTN)LocalBbsTable[Index].BootPriority,
>         (UINTN)LocalBbsTable[Index].Bus,
> --
> 2.28.0.windows.1
>

Re: [edk2-devel] [PATCH v3 2/8] FatPkg/FatPei: Remove extraneous debug message argument

[Michael Kubacki]

Hello Ray,

Can you please review this patch?

FatPkg is the only package missing a review on the series.

Thanks,
Michael

On 9/2/2022 6:22 PM, Michael Kubacki wrote:
> 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));
>     }

Re: [edk2-devel] [PATCH v3 2/8] FatPkg/FatPei: Remove extraneous debug message argument

[Michael Kubacki]

Still need this review.

Patch has been on list for over a month and changes one line of code.

On 9/6/2022 11:05 PM, Michael Kubacki wrote:
> Hello Ray,
> 
> Can you please review this patch?
> 
> FatPkg is the only package missing a review on the series.
> 
> Thanks,
> Michael
> 
> On 9/2/2022 6:22 PM, Michael Kubacki wrote:
>> 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));
>>     }
> 
> 
> 
> 

Re: [edk2-devel] [PATCH v3 2/8] FatPkg/FatPei: Remove extraneous debug message argument

[Michael D Kinney]

Reviewed-by: Michael D Kinney <michael.d.kinney@intel.com>

Mike

> -----Original Message-----
> From: devel@edk2.groups.io <devel@edk2.groups.io> On Behalf Of Michael Kubacki
> Sent: Thursday, September 8, 2022 9:28 AM
> To: Ni, Ray <ray.ni@intel.com>; devel@edk2.groups.io
> Subject: Re: [edk2-devel] [PATCH v3 2/8] FatPkg/FatPei: Remove extraneous debug message argument
> 
> Still need this review.
> 
> Patch has been on list for over a month and changes one line of code.
> 
> On 9/6/2022 11:05 PM, Michael Kubacki wrote:
> > Hello Ray,
> >
> > Can you please review this patch?
> >
> > FatPkg is the only package missing a review on the series.
> >
> > Thanks,
> > Michael
> >
> > On 9/2/2022 6:22 PM, Michael Kubacki wrote:
> >> 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));
> >>     }
> >
> >
> >
> >
> 
> 
> 
>