public inbox for devel@edk2.groups.io
 help / color / mirror / Atom feed
* [Patch 0/6] Reproduce builds across source format changes
@ 2021-11-01 18:29 Michael D Kinney
  2021-11-01 18:29 ` [Patch 1/6] MdePkg: " Michael D Kinney
                   ` (6 more replies)
  0 siblings, 7 replies; 10+ messages in thread
From: Michael D Kinney @ 2021-11-01 18:29 UTC (permalink / raw)
  To: devel

REF: https://bugzilla.tianocore.org/show_bug.cgi?id=3688

This change is required to help verify that source code formatting
changes such as the use of uncrustify and line ending corrections
do not have any functional differences. Source format changes may
add or remove line endings that change the source file line numbers
of C statements or may change the use of spaces in C expressions
used in an ASSERT() statements. These types of changes can impact
the generated binaries when DEBUG() and ASSERT() macros are
enabled. The following set of changes adds 2 defines that can be used
to override the use of __LINE__ in DEBUG() macros and the use of
#Expression in ASSERT() macros.

* Add DEBUG_LINE_NUMBER define to DebugLib.h that is
  by default mapped to __LINE__. A build can pre-define
  DEBUG_LINE_NUMBER to use a fixed value.
* Add DEBUG_EXPRESSION_STRING(Expression) macros to
  DebugLib.h that is by default mapped to #Expression.
  A build can define DEBUG_EXPRESSION_STRING_VALUE to
  set all expression strings to a fixed string value.
* Use DEBUG_LINE_NUMBER instead of __LINE__.
* Use DEBUG_EXPRESSION_STRING instead of #Expression.

Submodules that use __LINE__ are not updated. These do not
currently impact build reproducibility unless the debug features
of those submodules are enabled.

The one exception is the UnitTestFrameworkPkg cmocka submodule
that uses `__LINE__`.  This means that the binaries generated by host
based unit tests that use cmocka features may not be identical across
a source format change.

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: Michael Kubacki michael.kubacki@microsoft.com
Cc: Jian J Wang jian.j.wang@intel.com
Cc: Maciej Rabeda maciej.rabeda@linux.intel.com
Cc: Jiaxin Wu jiaxin.wu@intel.com
Cc: Siyuan Fu siyuan.fu@intel.com
Cc: Liming Gao gaoliming@byosoft.com.cn
Cc: Leif Lindholm leif@nuviainc.com
Cc: Zhiguang Liu zhiguang.liu@intel.com
Signed-off-by: Michael D Kinney michael.d.kinney@intel.com

Michael D Kinney (6):
  MdePkg: Reproduce builds across source format changes
  ArmPkg: Reproduce builds across source format changes
  MdeModulePkg: Reproduce builds across source format changes
  NetworkPkg: Reproduce builds across source format changes
  SecurityPkg: Reproduce builds across source format changes
  OvmfPkg: Reproduce builds across source format changes

 .../ProcessorSubClassDxe/ProcessorSubClass.c  |  2 +-
 .../Type00/MiscBiosVendorFunction.c           |  2 +-
 .../Type01/MiscSystemManufacturerFunction.c   |  2 +-
 .../MiscBaseBoardManufacturerFunction.c       |  2 +-
 .../Type03/MiscChassisManufacturerFunction.c  |  2 +-
 ...MiscNumberOfInstallableLanguagesFunction.c |  2 +-
 .../Type32/MiscBootInformationFunction.c      |  2 +-
 .../Bus/Pci/PciHostBridgeDxe/PciHostBridge.c  |  2 +-
 .../UefiBootManagerLib/BmDriverHealth.c       |  2 +-
 MdePkg/Include/Library/DebugLib.h             | 42 +++++++++++++++++--
 MdePkg/Include/Library/UnitTestLib.h          | 18 ++++----
 MdePkg/Library/BaseLib/SafeString.c           |  2 +-
 NetworkPkg/Include/Library/NetLib.h           |  8 ++--
 NetworkPkg/Library/DxeNetLib/DxeNetLib.c      |  2 +-
 OvmfPkg/Csm/LegacyBiosDxe/LegacyPci.c         |  6 +--
 .../PlatformBootManagerLib/BdsPlatform.c      |  4 +-
 .../PlatformBootManagerLibBhyve/BdsPlatform.c |  2 +-
 .../PlatformBootManagerLibGrub/BdsPlatform.c  |  4 +-
 .../Include/Library/TcgStorageCoreLib.h       |  4 +-
 19 files changed, 72 insertions(+), 38 deletions(-)

-- 
2.32.0.windows.1


^ permalink raw reply	[flat|nested] 10+ messages in thread
* [Patch 1/6] MdePkg: Reproduce builds across source format changes
  2021-11-01 18:29 [Patch 0/6] Reproduce builds across source format changes Michael D Kinney
@ 2021-11-01 18:29 ` Michael D Kinney
  2021-11-01 18:52   ` [edk2-devel] " Leif Lindholm
  2021-11-01 18:29 ` [Patch 2/6] ArmPkg: " Michael D Kinney
                   ` (5 subsequent siblings)
  6 siblings, 1 reply; 10+ messages in thread
From: Michael D Kinney @ 2021-11-01 18:29 UTC (permalink / raw)
  To: devel; +Cc: Liming Gao, Zhiguang Liu, Michael Kubacki

REF: https://bugzilla.tianocore.org/show_bug.cgi?id=3688

* Add DEBUG_LINE_NUMBER define to DebugLib.h that is
  by default mapped to __LINE__.  A build can pre-define
  DEBUG_LINE_NUMBER to use a fixed value.
* Add DEBUG_EXPRESSION_STRING(Expression) macrso to
  DebugLib.h that is by default mapped to #Expression.
  A build can define DEBUG_EXPRESSION_STRING_VALUE to
  set all expression strings to a fixed string value.
* Use DEBUG_LINE_NUMBER instead of __LINE__.
* Use DEBUG_EXPRESSION_STRING instead of #Expression.

Cc: Liming Gao <gaoliming@byosoft.com.cn>
Cc: Zhiguang Liu <zhiguang.liu@intel.com>
Cc: Michael Kubacki <michael.kubacki@microsoft.com>
Signed-off-by: Michael D Kinney <michael.d.kinney@intel.com>
---
 MdePkg/Include/Library/DebugLib.h    | 42 +++++++++++++++++++++++++---
 MdePkg/Include/Library/UnitTestLib.h | 18 ++++++------
 MdePkg/Library/BaseLib/SafeString.c  |  2 +-
 3 files changed, 48 insertions(+), 14 deletions(-)

diff --git a/MdePkg/Include/Library/DebugLib.h b/MdePkg/Include/Library/DebugLib.h
index 4cacd4b8e243..287b922e9f74 100644
--- a/MdePkg/Include/Library/DebugLib.h
+++ b/MdePkg/Include/Library/DebugLib.h
@@ -71,6 +71,40 @@ SPDX-License-Identifier: BSD-2-Clause-Patent
 #define EFI_D_VERBOSE   DEBUG_VERBOSE
 #define EFI_D_ERROR     DEBUG_ERROR
 
+//
+// Source file line number.
+// Default is use the to compiler provided __LINE__ macro value. The __LINE__
+// mapping can be overriden by predefining DEBUG_LINE_NUMBER
+//
+// Defining DEBUG_LINE_NUMBER to a fixed value is useful when comparing builds
+// across source code formatting changes that may add/remove lines in a source
+// file.
+//
+#ifndef DEBUG_LINE_NUMBER
+#define DEBUG_LINE_NUMBER  __LINE__
+#endif
+
+/**
+  Macro that converts a Boolean expression to a Null-terminated ASCII string.
+
+  The default is to use the C pre-processor stringizing operator '#' to add
+  quotes around the C expression. If DEBUG_EXPRESSION_STRING_VALUE is defined
+  then the C expression is converted to the fixed string value.
+
+  Defining DEBUG_EXPRESSION_STRING_VALUE to a fixed value is useful when
+  comparing builds across source code formatting changes that may make
+  changes to spaces or parenthesis in a Boolean expression.
+
+  @param  Expression  Boolean expression.
+
+**/
+
+#ifndef DEBUG_EXPRESSION_STRING_VALUE
+#define DEBUG_EXPRESSION_STRING(Expression)  #Expression
+#else
+#define DEBUG_EXPRESSION_STRING(Expression)  DEBUG_EXPRESSION_STRING_VALUE
+#endif
+
 /**
   Prints a debug message to the debug output device if the specified error level is enabled.
 
@@ -310,15 +344,15 @@ UnitTestDebugAssert (
   );
 
 #if defined(__clang__) && defined(__FILE_NAME__)
-#define _ASSERT(Expression)  UnitTestDebugAssert (__FILE_NAME__, __LINE__, #Expression)
+#define _ASSERT(Expression)  UnitTestDebugAssert (__FILE_NAME__, DEBUG_LINE_NUMBER, DEBUG_EXPRESSION_STRING(Expression))
 #else
-#define _ASSERT(Expression)  UnitTestDebugAssert (__FILE__, __LINE__, #Expression)
+#define _ASSERT(Expression)  UnitTestDebugAssert (__FILE__, DEBUG_LINE_NUMBER, DEBUG_EXPRESSION_STRING(Expression))
 #endif
 #else
 #if defined(__clang__) && defined(__FILE_NAME__)
-#define _ASSERT(Expression)  DebugAssert (__FILE_NAME__, __LINE__, #Expression)
+#define _ASSERT(Expression)  DebugAssert (__FILE_NAME__, DEBUG_LINE_NUMBER, DEBUG_EXPRESSION_STRING(Expression))
 #else
-#define _ASSERT(Expression)  DebugAssert (__FILE__, __LINE__, #Expression)
+#define _ASSERT(Expression)  DebugAssert (__FILE__, DEBUG_LINE_NUMBER, DEBUG_EXPRESSION_STRING(Expression))
 #endif
 #endif
 
diff --git a/MdePkg/Include/Library/UnitTestLib.h b/MdePkg/Include/Library/UnitTestLib.h
index 99175496c8cd..7cc6082387bb 100644
--- a/MdePkg/Include/Library/UnitTestLib.h
+++ b/MdePkg/Include/Library/UnitTestLib.h
@@ -348,7 +348,7 @@ SaveFrameworkState (
   @param[in]  Expression  Expression to be evaluated for TRUE.
 **/
 #define UT_ASSERT_TRUE(Expression)                                                        \
-  if(!UnitTestAssertTrue ((Expression), __FUNCTION__, __LINE__, __FILE__, #Expression)) { \
+  if(!UnitTestAssertTrue ((Expression), __FUNCTION__, DEBUG_LINE_NUMBER, __FILE__, #Expression)) { \
     return UNIT_TEST_ERROR_TEST_FAILED;                                                   \
   }
 
@@ -360,7 +360,7 @@ SaveFrameworkState (
   @param[in]  Expression  Expression to be evaluated for FALSE.
 **/
 #define UT_ASSERT_FALSE(Expression)                                                        \
-  if(!UnitTestAssertFalse ((Expression), __FUNCTION__, __LINE__, __FILE__, #Expression)) { \
+  if(!UnitTestAssertFalse ((Expression), __FUNCTION__, DEBUG_LINE_NUMBER, __FILE__, #Expression)) { \
     return UNIT_TEST_ERROR_TEST_FAILED;                                                    \
   }
 
@@ -373,7 +373,7 @@ SaveFrameworkState (
   @param[in]  ValueB  Value to be compared for equality (64-bit comparison).
 **/
 #define UT_ASSERT_EQUAL(ValueA, ValueB)                                                                           \
-  if(!UnitTestAssertEqual ((UINT64)(ValueA), (UINT64)(ValueB), __FUNCTION__, __LINE__, __FILE__, #ValueA, #ValueB)) { \
+  if(!UnitTestAssertEqual ((UINT64)(ValueA), (UINT64)(ValueB), __FUNCTION__, DEBUG_LINE_NUMBER, __FILE__, #ValueA, #ValueB)) { \
     return UNIT_TEST_ERROR_TEST_FAILED;                                                                           \
   }
 
@@ -387,7 +387,7 @@ SaveFrameworkState (
   @param[in]  Length   Number of bytes to compare in BufferA and BufferB.
 **/
 #define UT_ASSERT_MEM_EQUAL(BufferA, BufferB, Length)                                                                               \
-  if(!UnitTestAssertMemEqual ((VOID *)(UINTN)(BufferA), (VOID *)(UINTN)(BufferB), (UINTN)Length, __FUNCTION__, __LINE__, __FILE__, #BufferA, #BufferB)) { \
+  if(!UnitTestAssertMemEqual ((VOID *)(UINTN)(BufferA), (VOID *)(UINTN)(BufferB), (UINTN)Length, __FUNCTION__, DEBUG_LINE_NUMBER, __FILE__, #BufferA, #BufferB)) { \
     return UNIT_TEST_ERROR_TEST_FAILED;                                                                                           \
   }
 
@@ -400,7 +400,7 @@ SaveFrameworkState (
   @param[in]  ValueB  Value to be compared for inequality (64-bit comparison).
 **/
 #define UT_ASSERT_NOT_EQUAL(ValueA, ValueB)                                                                          \
-  if(!UnitTestAssertNotEqual ((UINT64)(ValueA), (UINT64)(ValueB), __FUNCTION__, __LINE__, __FILE__, #ValueA, #ValueB)) { \
+  if(!UnitTestAssertNotEqual ((UINT64)(ValueA), (UINT64)(ValueB), __FUNCTION__, DEBUG_LINE_NUMBER, __FILE__, #ValueA, #ValueB)) { \
     return UNIT_TEST_ERROR_TEST_FAILED;                                                                              \
   }
 
@@ -412,7 +412,7 @@ SaveFrameworkState (
   @param[in]  Status  EFI_STATUS value to check.
 **/
 #define UT_ASSERT_NOT_EFI_ERROR(Status)                                                \
-  if(!UnitTestAssertNotEfiError ((Status), __FUNCTION__, __LINE__, __FILE__, #Status)) { \
+  if(!UnitTestAssertNotEfiError ((Status), __FUNCTION__, DEBUG_LINE_NUMBER, __FILE__, #Status)) { \
     return UNIT_TEST_ERROR_TEST_FAILED;                                                \
   }
 
@@ -425,7 +425,7 @@ SaveFrameworkState (
   @param[in]  Expected  EFI_STATUS values to compare for equality.
 **/
 #define UT_ASSERT_STATUS_EQUAL(Status, Expected)                                                 \
-  if(!UnitTestAssertStatusEqual ((Status), (Expected), __FUNCTION__, __LINE__, __FILE__, #Status)) { \
+  if(!UnitTestAssertStatusEqual ((Status), (Expected), __FUNCTION__, DEBUG_LINE_NUMBER, __FILE__, #Status)) { \
     return UNIT_TEST_ERROR_TEST_FAILED;                                                          \
   }
 
@@ -437,7 +437,7 @@ SaveFrameworkState (
   @param[in]  Pointer  Pointer to be checked against NULL.
 **/
 #define UT_ASSERT_NOT_NULL(Pointer)                                                  \
-  if(!UnitTestAssertNotNull ((Pointer), __FUNCTION__, __LINE__, __FILE__, #Pointer)) { \
+  if(!UnitTestAssertNotNull ((Pointer), __FUNCTION__, DEBUG_LINE_NUMBER, __FILE__, #Pointer)) { \
     return UNIT_TEST_ERROR_TEST_FAILED;                                              \
   }
 
@@ -482,7 +482,7 @@ SaveFrameworkState (
       }                                                                \
       if (!UnitTestExpectAssertFailure (                               \
              UnitTestJumpStatus,                                       \
-             __FUNCTION__, __LINE__, __FILE__,                         \
+             __FUNCTION__, DEBUG_LINE_NUMBER, __FILE__,                    \
              #FunctionCall, Status)) {                                 \
         return UNIT_TEST_ERROR_TEST_FAILED;                            \
       }                                                                \
diff --git a/MdePkg/Library/BaseLib/SafeString.c b/MdePkg/Library/BaseLib/SafeString.c
index 3bb23ca1a130..ce6db2b94eff 100644
--- a/MdePkg/Library/BaseLib/SafeString.c
+++ b/MdePkg/Library/BaseLib/SafeString.c
@@ -17,7 +17,7 @@
     if (!(Expression)) { \
       DEBUG ((DEBUG_VERBOSE, \
         "%a(%d) %a: SAFE_STRING_CONSTRAINT_CHECK(%a) failed.  Return %r\n", \
-        __FILE__, __LINE__, __FUNCTION__, #Expression, Status)); \
+        __FILE__, DEBUG_LINE_NUMBER, __FUNCTION__, DEBUG_EXPRESSION_STRING(Expression), Status)); \
       return Status; \
     } \
   } while (FALSE)
-- 
2.32.0.windows.1


^ permalink raw reply related	[flat|nested] 10+ messages in thread
* [Patch 2/6] ArmPkg: Reproduce builds across source format changes
  2021-11-01 18:29 [Patch 0/6] Reproduce builds across source format changes Michael D Kinney
  2021-11-01 18:29 ` [Patch 1/6] MdePkg: " Michael D Kinney
@ 2021-11-01 18:29 ` Michael D Kinney
  2021-11-01 18:53   ` [edk2-devel] " Leif Lindholm
  2021-11-01 18:29 ` [Patch 3/6] MdeModulePkg: " Michael D Kinney
                   ` (4 subsequent siblings)
  6 siblings, 1 reply; 10+ messages in thread
From: Michael D Kinney @ 2021-11-01 18:29 UTC (permalink / raw)
  To: devel; +Cc: Leif Lindholm, Ard Biesheuvel, Michael Kubacki

REF: https://bugzilla.tianocore.org/show_bug.cgi?id=3688

Use DEBUG_LINE_NUMBER instead of __LINE__.

Cc: Leif Lindholm <leif@nuviainc.com>
Cc: Ard Biesheuvel <ardb+tianocore@kernel.org>
Cc: Michael Kubacki <michael.kubacki@microsoft.com>
Signed-off-by: Michael D Kinney <michael.d.kinney@intel.com>
---
 .../Universal/Smbios/ProcessorSubClassDxe/ProcessorSubClass.c   | 2 +-
 .../Smbios/SmbiosMiscDxe/Type00/MiscBiosVendorFunction.c        | 2 +-
 .../SmbiosMiscDxe/Type01/MiscSystemManufacturerFunction.c       | 2 +-
 .../SmbiosMiscDxe/Type02/MiscBaseBoardManufacturerFunction.c    | 2 +-
 .../SmbiosMiscDxe/Type03/MiscChassisManufacturerFunction.c      | 2 +-
 .../Type13/MiscNumberOfInstallableLanguagesFunction.c           | 2 +-
 .../Smbios/SmbiosMiscDxe/Type32/MiscBootInformationFunction.c   | 2 +-
 7 files changed, 7 insertions(+), 7 deletions(-)

diff --git a/ArmPkg/Universal/Smbios/ProcessorSubClassDxe/ProcessorSubClass.c b/ArmPkg/Universal/Smbios/ProcessorSubClassDxe/ProcessorSubClass.c
index 4b409ff7458c..cbdf6df01ee0 100644
--- a/ArmPkg/Universal/Smbios/ProcessorSubClassDxe/ProcessorSubClass.c
+++ b/ArmPkg/Universal/Smbios/ProcessorSubClassDxe/ProcessorSubClass.c
@@ -689,7 +689,7 @@ AddSmbiosProcessorTypeTable (
 
   if (EFI_ERROR (Status)) {
     DEBUG ((DEBUG_ERROR, "[%a]:[%dL] Smbios Type04 Table Log Failed! %r \n",
-            __FUNCTION__, __LINE__, Status));
+            __FUNCTION__, DEBUG_LINE_NUMBER, Status));
   }
   FreePool (Type4Record);
 
diff --git a/ArmPkg/Universal/Smbios/SmbiosMiscDxe/Type00/MiscBiosVendorFunction.c b/ArmPkg/Universal/Smbios/SmbiosMiscDxe/Type00/MiscBiosVendorFunction.c
index 5679ebaac8a5..2506c03d64b1 100644
--- a/ArmPkg/Universal/Smbios/SmbiosMiscDxe/Type00/MiscBiosVendorFunction.c
+++ b/ArmPkg/Universal/Smbios/SmbiosMiscDxe/Type00/MiscBiosVendorFunction.c
@@ -271,7 +271,7 @@ SMBIOS_MISC_TABLE_FUNCTION (MiscBiosVendor)
   Status = SmbiosMiscAddRecord ((UINT8*)SmbiosRecord, NULL);
   if (EFI_ERROR (Status)) {
     DEBUG ((DEBUG_ERROR, "[%a]:[%dL] Smbios Type00 Table Log Failed! %r \n",
-           __FUNCTION__, __LINE__, Status));
+           __FUNCTION__, DEBUG_LINE_NUMBER, Status));
   }
 
   FreePool (SmbiosRecord);
diff --git a/ArmPkg/Universal/Smbios/SmbiosMiscDxe/Type01/MiscSystemManufacturerFunction.c b/ArmPkg/Universal/Smbios/SmbiosMiscDxe/Type01/MiscSystemManufacturerFunction.c
index 2c69c2593f5d..555557a2a946 100644
--- a/ArmPkg/Universal/Smbios/SmbiosMiscDxe/Type01/MiscSystemManufacturerFunction.c
+++ b/ArmPkg/Universal/Smbios/SmbiosMiscDxe/Type01/MiscSystemManufacturerFunction.c
@@ -162,7 +162,7 @@ SMBIOS_MISC_TABLE_FUNCTION(MiscSystemManufacturer)
   Status = SmbiosMiscAddRecord ((UINT8*)SmbiosRecord, NULL);
   if (EFI_ERROR (Status)) {
     DEBUG ((DEBUG_ERROR, "[%a]:[%dL] Smbios Type01 Table Log Failed! %r \n",
-            __FUNCTION__, __LINE__, Status));
+            __FUNCTION__, DEBUG_LINE_NUMBER, Status));
   }
 
   FreePool (SmbiosRecord);
diff --git a/ArmPkg/Universal/Smbios/SmbiosMiscDxe/Type02/MiscBaseBoardManufacturerFunction.c b/ArmPkg/Universal/Smbios/SmbiosMiscDxe/Type02/MiscBaseBoardManufacturerFunction.c
index 097777a23904..f11295b199b3 100644
--- a/ArmPkg/Universal/Smbios/SmbiosMiscDxe/Type02/MiscBaseBoardManufacturerFunction.c
+++ b/ArmPkg/Universal/Smbios/SmbiosMiscDxe/Type02/MiscBaseBoardManufacturerFunction.c
@@ -196,7 +196,7 @@ SMBIOS_MISC_TABLE_FUNCTION(MiscBaseBoardManufacturer)
   Status = SmbiosMiscAddRecord ((UINT8 *)SmbiosRecord, NULL);
   if (EFI_ERROR (Status)) {
     DEBUG ((DEBUG_ERROR, "[%a]:[%dL] Smbios Type02 Table Log Failed! %r \n",
-            __FUNCTION__, __LINE__, Status));
+            __FUNCTION__, DEBUG_LINE_NUMBER, Status));
   }
 
   FreePool (SmbiosRecord);
diff --git a/ArmPkg/Universal/Smbios/SmbiosMiscDxe/Type03/MiscChassisManufacturerFunction.c b/ArmPkg/Universal/Smbios/SmbiosMiscDxe/Type03/MiscChassisManufacturerFunction.c
index 66e3e5327fc3..d64046182b96 100644
--- a/ArmPkg/Universal/Smbios/SmbiosMiscDxe/Type03/MiscChassisManufacturerFunction.c
+++ b/ArmPkg/Universal/Smbios/SmbiosMiscDxe/Type03/MiscChassisManufacturerFunction.c
@@ -178,7 +178,7 @@ SMBIOS_MISC_TABLE_FUNCTION(MiscChassisManufacturer)
   Status = SmbiosMiscAddRecord ((UINT8*)SmbiosRecord, NULL);
   if (EFI_ERROR (Status)) {
     DEBUG ((DEBUG_ERROR, "[%a]:[%dL] Smbios Type03 Table Log Failed! %r \n",
-            __FUNCTION__, __LINE__, Status));
+            __FUNCTION__, DEBUG_LINE_NUMBER, Status));
   }
 
   FreePool (SmbiosRecord);
diff --git a/ArmPkg/Universal/Smbios/SmbiosMiscDxe/Type13/MiscNumberOfInstallableLanguagesFunction.c b/ArmPkg/Universal/Smbios/SmbiosMiscDxe/Type13/MiscNumberOfInstallableLanguagesFunction.c
index 7c941b5c0709..017b410a16d0 100644
--- a/ArmPkg/Universal/Smbios/SmbiosMiscDxe/Type13/MiscNumberOfInstallableLanguagesFunction.c
+++ b/ArmPkg/Universal/Smbios/SmbiosMiscDxe/Type13/MiscNumberOfInstallableLanguagesFunction.c
@@ -158,7 +158,7 @@ SMBIOS_MISC_TABLE_FUNCTION(MiscNumberOfInstallableLanguages)
   Status = SmbiosMiscAddRecord ((UINT8*)SmbiosRecord, NULL);
   if (EFI_ERROR (Status)) {
     DEBUG ((DEBUG_ERROR, "[%a]:[%dL] Smbios Type13 Table Log Failed! %r \n",
-            __FUNCTION__, __LINE__, Status));
+            __FUNCTION__, DEBUG_LINE_NUMBER, Status));
   }
 
   FreePool (SmbiosRecord);
diff --git a/ArmPkg/Universal/Smbios/SmbiosMiscDxe/Type32/MiscBootInformationFunction.c b/ArmPkg/Universal/Smbios/SmbiosMiscDxe/Type32/MiscBootInformationFunction.c
index 4be1e1cd29a9..c4ce6a5e7608 100644
--- a/ArmPkg/Universal/Smbios/SmbiosMiscDxe/Type32/MiscBootInformationFunction.c
+++ b/ArmPkg/Universal/Smbios/SmbiosMiscDxe/Type32/MiscBootInformationFunction.c
@@ -68,7 +68,7 @@ SMBIOS_MISC_TABLE_FUNCTION(MiscBootInformation)
   Status = SmbiosMiscAddRecord ((UINT8*)SmbiosRecord, NULL);
   if (EFI_ERROR (Status)) {
     DEBUG ((DEBUG_ERROR, "[%a]:[%dL] Smbios Type32 Table Log Failed! %r \n",
-            __FUNCTION__, __LINE__, Status));
+            __FUNCTION__, DEBUG_LINE_NUMBER, Status));
   }
 
   FreePool (SmbiosRecord);
-- 
2.32.0.windows.1


^ permalink raw reply related	[flat|nested] 10+ messages in thread
* [Patch 3/6] MdeModulePkg: Reproduce builds across source format changes
  2021-11-01 18:29 [Patch 0/6] Reproduce builds across source format changes Michael D Kinney
  2021-11-01 18:29 ` [Patch 1/6] MdePkg: " Michael D Kinney
  2021-11-01 18:29 ` [Patch 2/6] ArmPkg: " Michael D Kinney
@ 2021-11-01 18:29 ` Michael D Kinney
  2021-11-01 18:29 ` [Patch 4/6] NetworkPkg: " Michael D Kinney
                   ` (3 subsequent siblings)
  6 siblings, 0 replies; 10+ messages in thread
From: Michael D Kinney @ 2021-11-01 18:29 UTC (permalink / raw)
  To: devel; +Cc: Jian J Wang, Liming Gao, Michael Kubacki

REF: https://bugzilla.tianocore.org/show_bug.cgi?id=3688

Use DEBUG_LINE_NUMBER instead of __LINE__.

Cc: Jian J Wang <jian.j.wang@intel.com>
Cc: Liming Gao <gaoliming@byosoft.com.cn>
Cc: Michael Kubacki <michael.kubacki@microsoft.com>
Signed-off-by: Michael D Kinney <michael.d.kinney@intel.com>
---
 MdeModulePkg/Bus/Pci/PciHostBridgeDxe/PciHostBridge.c    | 2 +-
 MdeModulePkg/Library/UefiBootManagerLib/BmDriverHealth.c | 2 +-
 2 files changed, 2 insertions(+), 2 deletions(-)

diff --git a/MdeModulePkg/Bus/Pci/PciHostBridgeDxe/PciHostBridge.c b/MdeModulePkg/Bus/Pci/PciHostBridgeDxe/PciHostBridge.c
index 4ab9415c9600..163e931dd9e8 100644
--- a/MdeModulePkg/Bus/Pci/PciHostBridgeDxe/PciHostBridge.c
+++ b/MdeModulePkg/Bus/Pci/PciHostBridgeDxe/PciHostBridge.c
@@ -874,7 +874,7 @@ NotifyPhase (
           Translation = GetTranslationByResourceType (RootBridge, Index);
           if ((Translation & Alignment) != 0) {
             DEBUG ((DEBUG_ERROR, "[%a:%d] Translation %lx is not aligned to %lx!\n",
-              __FUNCTION__, __LINE__, Translation, Alignment
+              __FUNCTION__, DEBUG_LINE_NUMBER, Translation, Alignment
               ));
             ASSERT ((Translation & Alignment) == 0);
             //
diff --git a/MdeModulePkg/Library/UefiBootManagerLib/BmDriverHealth.c b/MdeModulePkg/Library/UefiBootManagerLib/BmDriverHealth.c
index 52e35e45185d..fa1fd87d15ef 100644
--- a/MdeModulePkg/Library/UefiBootManagerLib/BmDriverHealth.c
+++ b/MdeModulePkg/Library/UefiBootManagerLib/BmDriverHealth.c
@@ -574,7 +574,7 @@ BmRepairAllControllers (
       BmRepairAllControllers (ReconnectRepairCount + 1);
     } else {
       DEBUG ((DEBUG_ERROR, "[%a:%d] Repair failed after %d retries.\n",
-        __FUNCTION__, __LINE__, ReconnectRepairCount));
+        __FUNCTION__, DEBUG_LINE_NUMBER, ReconnectRepairCount));
     }
   }
 
-- 
2.32.0.windows.1


^ permalink raw reply related	[flat|nested] 10+ messages in thread
* [Patch 4/6] NetworkPkg: Reproduce builds across source format changes
  2021-11-01 18:29 [Patch 0/6] Reproduce builds across source format changes Michael D Kinney
                   ` (2 preceding siblings ...)
  2021-11-01 18:29 ` [Patch 3/6] MdeModulePkg: " Michael D Kinney
@ 2021-11-01 18:29 ` Michael D Kinney
  2021-11-01 18:29 ` [Patch 5/6] SecurityPkg: " Michael D Kinney
                   ` (2 subsequent siblings)
  6 siblings, 0 replies; 10+ messages in thread
From: Michael D Kinney @ 2021-11-01 18:29 UTC (permalink / raw)
  To: devel; +Cc: Maciej Rabeda, Jiaxin Wu, Siyuan Fu, Michael Kubacki

REF: https://bugzilla.tianocore.org/show_bug.cgi?id=3688

Use DEBUG_LINE_NUMBER instead of __LINE__.

Cc: Maciej Rabeda <maciej.rabeda@linux.intel.com>
Cc: Jiaxin Wu <jiaxin.wu@intel.com>
Cc: Siyuan Fu <siyuan.fu@intel.com>
Cc: Michael Kubacki <michael.kubacki@microsoft.com>
Signed-off-by: Michael D Kinney <michael.d.kinney@intel.com>
---
 NetworkPkg/Include/Library/NetLib.h      | 8 ++++----
 NetworkPkg/Library/DxeNetLib/DxeNetLib.c | 2 +-
 2 files changed, 5 insertions(+), 5 deletions(-)

diff --git a/NetworkPkg/Include/Library/NetLib.h b/NetworkPkg/Include/Library/NetLib.h
index 858d0b6ba07c..6c0924863147 100644
--- a/NetworkPkg/Include/Library/NetLib.h
+++ b/NetworkPkg/Include/Library/NetLib.h
@@ -277,7 +277,7 @@ typedef struct {
     NETDEBUG_LEVEL_TRACE, \
     Module, \
     __FILE__, \
-    __LINE__, \
+    DEBUG_LINE_NUMBER, \
     NetDebugASPrint PrintArg \
     )
 
@@ -286,7 +286,7 @@ typedef struct {
     NETDEBUG_LEVEL_WARNING, \
     Module, \
     __FILE__, \
-    __LINE__, \
+    DEBUG_LINE_NUMBER, \
     NetDebugASPrint PrintArg \
     )
 
@@ -295,7 +295,7 @@ typedef struct {
     NETDEBUG_LEVEL_ERROR, \
     Module, \
     __FILE__, \
-    __LINE__, \
+    DEBUG_LINE_NUMBER, \
     NetDebugASPrint PrintArg \
     )
 
@@ -311,7 +311,7 @@ typedef struct {
            NETDEBUG_LEVEL_TRACE,
            "Tcp",
            __FILE__,
-           __LINE__,
+           DEBUG_LINE_NUMBER,
            NetDebugASPrint ("State transit to %a\n", Name)
          )
 
diff --git a/NetworkPkg/Library/DxeNetLib/DxeNetLib.c b/NetworkPkg/Library/DxeNetLib/DxeNetLib.c
index 2a555a7b90fa..0f95ce4b710d 100644
--- a/NetworkPkg/Library/DxeNetLib/DxeNetLib.c
+++ b/NetworkPkg/Library/DxeNetLib/DxeNetLib.c
@@ -434,7 +434,7 @@ SyslogBuildPacket (
            NETDEBUG_LEVEL_TRACE,
            "Tcp",
            __FILE__,
-           __LINE__,
+           DEBUG_LINE_NUMBER,
            NetDebugASPrint ("State transit to %a\n", Name)
          )
 
-- 
2.32.0.windows.1


^ permalink raw reply related	[flat|nested] 10+ messages in thread
* [Patch 5/6] SecurityPkg: Reproduce builds across source format changes
  2021-11-01 18:29 [Patch 0/6] Reproduce builds across source format changes Michael D Kinney
                   ` (3 preceding siblings ...)
  2021-11-01 18:29 ` [Patch 4/6] NetworkPkg: " Michael D Kinney
@ 2021-11-01 18:29 ` Michael D Kinney
  2021-11-01 18:29 ` [Patch 6/6] OvmfPkg: " Michael D Kinney
  2021-11-01 19:03 ` [edk2-devel] [Patch 0/6] " Michael Kubacki
  6 siblings, 0 replies; 10+ messages in thread
From: Michael D Kinney @ 2021-11-01 18:29 UTC (permalink / raw)
  To: devel; +Cc: Jiewen Yao, Jian J Wang, Michael Kubacki

REF: https://bugzilla.tianocore.org/show_bug.cgi?id=3688

Use DEBUG_LINE_NUMBER instead of __LINE__.

Cc: Jiewen Yao <jiewen.yao@intel.com>
Cc: Jian J Wang <jian.j.wang@intel.com>
Cc: Michael Kubacki <michael.kubacki@microsoft.com>
Signed-off-by: Michael D Kinney <michael.d.kinney@intel.com>
---
 SecurityPkg/Include/Library/TcgStorageCoreLib.h | 4 ++--
 1 file changed, 2 insertions(+), 2 deletions(-)

diff --git a/SecurityPkg/Include/Library/TcgStorageCoreLib.h b/SecurityPkg/Include/Library/TcgStorageCoreLib.h
index 01a44c667c80..44dbdc0ffa32 100644
--- a/SecurityPkg/Include/Library/TcgStorageCoreLib.h
+++ b/SecurityPkg/Include/Library/TcgStorageCoreLib.h
@@ -20,7 +20,7 @@ SPDX-License-Identifier: BSD-2-Clause-Patent
   {                                                                              \
     TCG_RESULT ret = (arg);                                                      \
     if (ret != TcgResultSuccess) {                                               \
-      DEBUG ((DEBUG_INFO, "ERROR_CHECK failed at %a:%u\n", __FILE__, __LINE__)); \
+      DEBUG ((DEBUG_INFO, "ERROR_CHECK failed at %a:%u\n", __FILE__, DEBUG_LINE_NUMBER)); \
       return ret;                                                                \
     }                                                                            \
   }
@@ -34,7 +34,7 @@ SPDX-License-Identifier: BSD-2-Clause-Patent
 #define NULL_CHECK(arg)                                                                   \
   do {                                                                                    \
     if ((arg) == NULL) {                                                                  \
-      DEBUG ((DEBUG_INFO, "NULL_CHECK(%a) failed at %a:%u\n", #arg, __FILE__, __LINE__)); \
+      DEBUG ((DEBUG_INFO, "NULL_CHECK(%a) failed at %a:%u\n", #arg, __FILE__, DEBUG_LINE_NUMBER)); \
       return TcgResultFailureNullPointer;                                                 \
     }                                                                                     \
   } while (0)
-- 
2.32.0.windows.1


^ permalink raw reply related	[flat|nested] 10+ messages in thread
* [Patch 6/6] OvmfPkg: Reproduce builds across source format changes
  2021-11-01 18:29 [Patch 0/6] Reproduce builds across source format changes Michael D Kinney
                   ` (4 preceding siblings ...)
  2021-11-01 18:29 ` [Patch 5/6] SecurityPkg: " Michael D Kinney
@ 2021-11-01 18:29 ` Michael D Kinney
  2021-11-01 19:03 ` [edk2-devel] [Patch 0/6] " Michael Kubacki
  6 siblings, 0 replies; 10+ messages in thread
From: Michael D Kinney @ 2021-11-01 18:29 UTC (permalink / raw)
  To: devel
  Cc: Ard Biesheuvel, Jiewen Yao, Jordan Justen, Gerd Hoffmann,
	Michael Kubacki

REF: https://bugzilla.tianocore.org/show_bug.cgi?id=3688

Use DEBUG_LINE_NUMBER instead of __LINE__.

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: Michael Kubacki <michael.kubacki@microsoft.com>
Signed-off-by: Michael D Kinney <michael.d.kinney@intel.com>
---
 OvmfPkg/Csm/LegacyBiosDxe/LegacyPci.c                     | 6 +++---
 OvmfPkg/Library/PlatformBootManagerLib/BdsPlatform.c      | 4 ++--
 OvmfPkg/Library/PlatformBootManagerLibBhyve/BdsPlatform.c | 2 +-
 OvmfPkg/Library/PlatformBootManagerLibGrub/BdsPlatform.c  | 4 ++--
 4 files changed, 8 insertions(+), 8 deletions(-)

diff --git a/OvmfPkg/Csm/LegacyBiosDxe/LegacyPci.c b/OvmfPkg/Csm/LegacyBiosDxe/LegacyPci.c
index 746b366448e6..350cf3dd0b3a 100644
--- a/OvmfPkg/Csm/LegacyBiosDxe/LegacyPci.c
+++ b/OvmfPkg/Csm/LegacyBiosDxe/LegacyPci.c
@@ -2321,7 +2321,7 @@ LegacyBiosInstallRom (
                     );
 
     if (EFI_ERROR (Status)) {
-      DEBUG ((DEBUG_ERROR, "return LegacyBiosInstallRom(%d): EFI_OUT_OF_RESOURCES (no more space for OpROM)\n", __LINE__));
+      DEBUG ((DEBUG_ERROR, "return LegacyBiosInstallRom(%d): EFI_OUT_OF_RESOURCES (no more space for OpROM)\n", DEBUG_LINE_NUMBER));
       //
       // Report Status Code to indicate that there is no enough space for OpROM
       //
@@ -2337,7 +2337,7 @@ LegacyBiosInstallRom (
     //
     RuntimeAddress = Private->OptionRom;
     if (RuntimeAddress + *RuntimeImageLength > MaxRomAddr) {
-      DEBUG ((DEBUG_ERROR, "return LegacyBiosInstallRom(%d): EFI_OUT_OF_RESOURCES (no more space for OpROM)\n", __LINE__));
+      DEBUG ((DEBUG_ERROR, "return LegacyBiosInstallRom(%d): EFI_OUT_OF_RESOURCES (no more space for OpROM)\n", DEBUG_LINE_NUMBER));
       gBS->FreePages (PhysicalAddress, EFI_SIZE_TO_PAGES (ImageSize));
       //
       // Report Status Code to indicate that there is no enough space for OpROM
@@ -2355,7 +2355,7 @@ LegacyBiosInstallRom (
     //
     InitAddress    = PCI_START_ADDRESS (Private->OptionRom);
     if (InitAddress + ImageSize > MaxRomAddr) {
-      DEBUG ((DEBUG_ERROR, "return LegacyBiosInstallRom(%d): EFI_OUT_OF_RESOURCES (no more space for OpROM)\n", __LINE__));
+      DEBUG ((DEBUG_ERROR, "return LegacyBiosInstallRom(%d): EFI_OUT_OF_RESOURCES (no more space for OpROM)\n", DEBUG_LINE_NUMBER));
       //
       // Report Status Code to indicate that there is no enough space for OpROM
       //
diff --git a/OvmfPkg/Library/PlatformBootManagerLib/BdsPlatform.c b/OvmfPkg/Library/PlatformBootManagerLib/BdsPlatform.c
index 9b21ba2bd699..186401296ae2 100644
--- a/OvmfPkg/Library/PlatformBootManagerLib/BdsPlatform.c
+++ b/OvmfPkg/Library/PlatformBootManagerLib/BdsPlatform.c
@@ -624,7 +624,7 @@ PrepareLpcBridgeDevicePath (
     DEBUG((
       DEBUG_INFO,
       "BdsPlatform.c+%d: COM%d DevPath: %s\n",
-      __LINE__,
+      DEBUG_LINE_NUMBER,
       gPnp16550ComPortDeviceNode.UID + 1,
       DevPathStr
       ));
@@ -656,7 +656,7 @@ PrepareLpcBridgeDevicePath (
     DEBUG((
       DEBUG_INFO,
       "BdsPlatform.c+%d: COM%d DevPath: %s\n",
-      __LINE__,
+      DEBUG_LINE_NUMBER,
       gPnp16550ComPortDeviceNode.UID + 1,
       DevPathStr
       ));
diff --git a/OvmfPkg/Library/PlatformBootManagerLibBhyve/BdsPlatform.c b/OvmfPkg/Library/PlatformBootManagerLibBhyve/BdsPlatform.c
index 513d2f00a747..e767c3b172ba 100644
--- a/OvmfPkg/Library/PlatformBootManagerLibBhyve/BdsPlatform.c
+++ b/OvmfPkg/Library/PlatformBootManagerLibBhyve/BdsPlatform.c
@@ -586,7 +586,7 @@ PrepareLpcBridgeDevicePath (
     DEBUG((
       DEBUG_INFO,
       "BdsPlatform.c+%d: COM%d DevPath: %s\n",
-      __LINE__,
+      DEBUG_LINE_NUMBER,
       gPnp16550ComPortDeviceNode.UID + 1,
       DevPathStr
       ));
diff --git a/OvmfPkg/Library/PlatformBootManagerLibGrub/BdsPlatform.c b/OvmfPkg/Library/PlatformBootManagerLibGrub/BdsPlatform.c
index 1c5405f620e7..fd8057735549 100644
--- a/OvmfPkg/Library/PlatformBootManagerLibGrub/BdsPlatform.c
+++ b/OvmfPkg/Library/PlatformBootManagerLibGrub/BdsPlatform.c
@@ -556,7 +556,7 @@ PrepareLpcBridgeDevicePath (
     DEBUG((
       DEBUG_INFO,
       "BdsPlatform.c+%d: COM%d DevPath: %s\n",
-      __LINE__,
+      DEBUG_LINE_NUMBER,
       gPnp16550ComPortDeviceNode.UID + 1,
       DevPathStr
       ));
@@ -588,7 +588,7 @@ PrepareLpcBridgeDevicePath (
     DEBUG((
       DEBUG_INFO,
       "BdsPlatform.c+%d: COM%d DevPath: %s\n",
-      __LINE__,
+      DEBUG_LINE_NUMBER,
       gPnp16550ComPortDeviceNode.UID + 1,
       DevPathStr
       ));
-- 
2.32.0.windows.1


^ permalink raw reply related	[flat|nested] 10+ messages in thread
* Re: [edk2-devel] [Patch 1/6] MdePkg: Reproduce builds across source format changes
  2021-11-01 18:29 ` [Patch 1/6] MdePkg: " Michael D Kinney
@ 2021-11-01 18:52   ` Leif Lindholm
  0 siblings, 0 replies; 10+ messages in thread
From: Leif Lindholm @ 2021-11-01 18:52 UTC (permalink / raw)
  To: devel, michael.d.kinney; +Cc: Liming Gao, Zhiguang Liu, Michael Kubacki

On Mon, Nov 01, 2021 at 11:29:37 -0700, Michael D Kinney wrote:
> REF: https://bugzilla.tianocore.org/show_bug.cgi?id=3688
> 
> * Add DEBUG_LINE_NUMBER define to DebugLib.h that is
>   by default mapped to __LINE__.  A build can pre-define
>   DEBUG_LINE_NUMBER to use a fixed value.
> * Add DEBUG_EXPRESSION_STRING(Expression) macrso to
>   DebugLib.h that is by default mapped to #Expression.
>   A build can define DEBUG_EXPRESSION_STRING_VALUE to
>   set all expression strings to a fixed string value.

I think it would be useful to separate this


> * Use DEBUG_LINE_NUMBER instead of __LINE__.
> * Use DEBUG_EXPRESSION_STRING instead of #Expression.

from this.

I.e., I have no need to verify whether macro use changes in MdePkg are
correct, but it is interesting to me to see the new macros being
defined.

/
    Leif

> Cc: Liming Gao <gaoliming@byosoft.com.cn>
> Cc: Zhiguang Liu <zhiguang.liu@intel.com>
> Cc: Michael Kubacki <michael.kubacki@microsoft.com>
> Signed-off-by: Michael D Kinney <michael.d.kinney@intel.com>
> ---
>  MdePkg/Include/Library/DebugLib.h    | 42 +++++++++++++++++++++++++---
>  MdePkg/Include/Library/UnitTestLib.h | 18 ++++++------
>  MdePkg/Library/BaseLib/SafeString.c  |  2 +-
>  3 files changed, 48 insertions(+), 14 deletions(-)
> 
> diff --git a/MdePkg/Include/Library/DebugLib.h b/MdePkg/Include/Library/DebugLib.h
> index 4cacd4b8e243..287b922e9f74 100644
> --- a/MdePkg/Include/Library/DebugLib.h
> +++ b/MdePkg/Include/Library/DebugLib.h
> @@ -71,6 +71,40 @@ SPDX-License-Identifier: BSD-2-Clause-Patent
>  #define EFI_D_VERBOSE   DEBUG_VERBOSE
>  #define EFI_D_ERROR     DEBUG_ERROR
>  
> +//
> +// Source file line number.
> +// Default is use the to compiler provided __LINE__ macro value. The __LINE__
> +// mapping can be overriden by predefining DEBUG_LINE_NUMBER
> +//
> +// Defining DEBUG_LINE_NUMBER to a fixed value is useful when comparing builds
> +// across source code formatting changes that may add/remove lines in a source
> +// file.
> +//
> +#ifndef DEBUG_LINE_NUMBER
> +#define DEBUG_LINE_NUMBER  __LINE__
> +#endif
> +
> +/**
> +  Macro that converts a Boolean expression to a Null-terminated ASCII string.
> +
> +  The default is to use the C pre-processor stringizing operator '#' to add
> +  quotes around the C expression. If DEBUG_EXPRESSION_STRING_VALUE is defined
> +  then the C expression is converted to the fixed string value.
> +
> +  Defining DEBUG_EXPRESSION_STRING_VALUE to a fixed value is useful when
> +  comparing builds across source code formatting changes that may make
> +  changes to spaces or parenthesis in a Boolean expression.
> +
> +  @param  Expression  Boolean expression.
> +
> +**/
> +
> +#ifndef DEBUG_EXPRESSION_STRING_VALUE
> +#define DEBUG_EXPRESSION_STRING(Expression)  #Expression
> +#else
> +#define DEBUG_EXPRESSION_STRING(Expression)  DEBUG_EXPRESSION_STRING_VALUE
> +#endif
> +
>  /**
>    Prints a debug message to the debug output device if the specified error level is enabled.
>  
> @@ -310,15 +344,15 @@ UnitTestDebugAssert (
>    );
>  
>  #if defined(__clang__) && defined(__FILE_NAME__)
> -#define _ASSERT(Expression)  UnitTestDebugAssert (__FILE_NAME__, __LINE__, #Expression)
> +#define _ASSERT(Expression)  UnitTestDebugAssert (__FILE_NAME__, DEBUG_LINE_NUMBER, DEBUG_EXPRESSION_STRING(Expression))
>  #else
> -#define _ASSERT(Expression)  UnitTestDebugAssert (__FILE__, __LINE__, #Expression)
> +#define _ASSERT(Expression)  UnitTestDebugAssert (__FILE__, DEBUG_LINE_NUMBER, DEBUG_EXPRESSION_STRING(Expression))
>  #endif
>  #else
>  #if defined(__clang__) && defined(__FILE_NAME__)
> -#define _ASSERT(Expression)  DebugAssert (__FILE_NAME__, __LINE__, #Expression)
> +#define _ASSERT(Expression)  DebugAssert (__FILE_NAME__, DEBUG_LINE_NUMBER, DEBUG_EXPRESSION_STRING(Expression))
>  #else
> -#define _ASSERT(Expression)  DebugAssert (__FILE__, __LINE__, #Expression)
> +#define _ASSERT(Expression)  DebugAssert (__FILE__, DEBUG_LINE_NUMBER, DEBUG_EXPRESSION_STRING(Expression))
>  #endif
>  #endif
>  
> diff --git a/MdePkg/Include/Library/UnitTestLib.h b/MdePkg/Include/Library/UnitTestLib.h
> index 99175496c8cd..7cc6082387bb 100644
> --- a/MdePkg/Include/Library/UnitTestLib.h
> +++ b/MdePkg/Include/Library/UnitTestLib.h
> @@ -348,7 +348,7 @@ SaveFrameworkState (
>    @param[in]  Expression  Expression to be evaluated for TRUE.
>  **/
>  #define UT_ASSERT_TRUE(Expression)                                                        \
> -  if(!UnitTestAssertTrue ((Expression), __FUNCTION__, __LINE__, __FILE__, #Expression)) { \
> +  if(!UnitTestAssertTrue ((Expression), __FUNCTION__, DEBUG_LINE_NUMBER, __FILE__, #Expression)) { \
>      return UNIT_TEST_ERROR_TEST_FAILED;                                                   \
>    }
>  
> @@ -360,7 +360,7 @@ SaveFrameworkState (
>    @param[in]  Expression  Expression to be evaluated for FALSE.
>  **/
>  #define UT_ASSERT_FALSE(Expression)                                                        \
> -  if(!UnitTestAssertFalse ((Expression), __FUNCTION__, __LINE__, __FILE__, #Expression)) { \
> +  if(!UnitTestAssertFalse ((Expression), __FUNCTION__, DEBUG_LINE_NUMBER, __FILE__, #Expression)) { \
>      return UNIT_TEST_ERROR_TEST_FAILED;                                                    \
>    }
>  
> @@ -373,7 +373,7 @@ SaveFrameworkState (
>    @param[in]  ValueB  Value to be compared for equality (64-bit comparison).
>  **/
>  #define UT_ASSERT_EQUAL(ValueA, ValueB)                                                                           \
> -  if(!UnitTestAssertEqual ((UINT64)(ValueA), (UINT64)(ValueB), __FUNCTION__, __LINE__, __FILE__, #ValueA, #ValueB)) { \
> +  if(!UnitTestAssertEqual ((UINT64)(ValueA), (UINT64)(ValueB), __FUNCTION__, DEBUG_LINE_NUMBER, __FILE__, #ValueA, #ValueB)) { \
>      return UNIT_TEST_ERROR_TEST_FAILED;                                                                           \
>    }
>  
> @@ -387,7 +387,7 @@ SaveFrameworkState (
>    @param[in]  Length   Number of bytes to compare in BufferA and BufferB.
>  **/
>  #define UT_ASSERT_MEM_EQUAL(BufferA, BufferB, Length)                                                                               \
> -  if(!UnitTestAssertMemEqual ((VOID *)(UINTN)(BufferA), (VOID *)(UINTN)(BufferB), (UINTN)Length, __FUNCTION__, __LINE__, __FILE__, #BufferA, #BufferB)) { \
> +  if(!UnitTestAssertMemEqual ((VOID *)(UINTN)(BufferA), (VOID *)(UINTN)(BufferB), (UINTN)Length, __FUNCTION__, DEBUG_LINE_NUMBER, __FILE__, #BufferA, #BufferB)) { \
>      return UNIT_TEST_ERROR_TEST_FAILED;                                                                                           \
>    }
>  
> @@ -400,7 +400,7 @@ SaveFrameworkState (
>    @param[in]  ValueB  Value to be compared for inequality (64-bit comparison).
>  **/
>  #define UT_ASSERT_NOT_EQUAL(ValueA, ValueB)                                                                          \
> -  if(!UnitTestAssertNotEqual ((UINT64)(ValueA), (UINT64)(ValueB), __FUNCTION__, __LINE__, __FILE__, #ValueA, #ValueB)) { \
> +  if(!UnitTestAssertNotEqual ((UINT64)(ValueA), (UINT64)(ValueB), __FUNCTION__, DEBUG_LINE_NUMBER, __FILE__, #ValueA, #ValueB)) { \
>      return UNIT_TEST_ERROR_TEST_FAILED;                                                                              \
>    }
>  
> @@ -412,7 +412,7 @@ SaveFrameworkState (
>    @param[in]  Status  EFI_STATUS value to check.
>  **/
>  #define UT_ASSERT_NOT_EFI_ERROR(Status)                                                \
> -  if(!UnitTestAssertNotEfiError ((Status), __FUNCTION__, __LINE__, __FILE__, #Status)) { \
> +  if(!UnitTestAssertNotEfiError ((Status), __FUNCTION__, DEBUG_LINE_NUMBER, __FILE__, #Status)) { \
>      return UNIT_TEST_ERROR_TEST_FAILED;                                                \
>    }
>  
> @@ -425,7 +425,7 @@ SaveFrameworkState (
>    @param[in]  Expected  EFI_STATUS values to compare for equality.
>  **/
>  #define UT_ASSERT_STATUS_EQUAL(Status, Expected)                                                 \
> -  if(!UnitTestAssertStatusEqual ((Status), (Expected), __FUNCTION__, __LINE__, __FILE__, #Status)) { \
> +  if(!UnitTestAssertStatusEqual ((Status), (Expected), __FUNCTION__, DEBUG_LINE_NUMBER, __FILE__, #Status)) { \
>      return UNIT_TEST_ERROR_TEST_FAILED;                                                          \
>    }
>  
> @@ -437,7 +437,7 @@ SaveFrameworkState (
>    @param[in]  Pointer  Pointer to be checked against NULL.
>  **/
>  #define UT_ASSERT_NOT_NULL(Pointer)                                                  \
> -  if(!UnitTestAssertNotNull ((Pointer), __FUNCTION__, __LINE__, __FILE__, #Pointer)) { \
> +  if(!UnitTestAssertNotNull ((Pointer), __FUNCTION__, DEBUG_LINE_NUMBER, __FILE__, #Pointer)) { \
>      return UNIT_TEST_ERROR_TEST_FAILED;                                              \
>    }
>  
> @@ -482,7 +482,7 @@ SaveFrameworkState (
>        }                                                                \
>        if (!UnitTestExpectAssertFailure (                               \
>               UnitTestJumpStatus,                                       \
> -             __FUNCTION__, __LINE__, __FILE__,                         \
> +             __FUNCTION__, DEBUG_LINE_NUMBER, __FILE__,                    \
>               #FunctionCall, Status)) {                                 \
>          return UNIT_TEST_ERROR_TEST_FAILED;                            \
>        }                                                                \
> diff --git a/MdePkg/Library/BaseLib/SafeString.c b/MdePkg/Library/BaseLib/SafeString.c
> index 3bb23ca1a130..ce6db2b94eff 100644
> --- a/MdePkg/Library/BaseLib/SafeString.c
> +++ b/MdePkg/Library/BaseLib/SafeString.c
> @@ -17,7 +17,7 @@
>      if (!(Expression)) { \
>        DEBUG ((DEBUG_VERBOSE, \
>          "%a(%d) %a: SAFE_STRING_CONSTRAINT_CHECK(%a) failed.  Return %r\n", \
> -        __FILE__, __LINE__, __FUNCTION__, #Expression, Status)); \
> +        __FILE__, DEBUG_LINE_NUMBER, __FUNCTION__, DEBUG_EXPRESSION_STRING(Expression), Status)); \
>        return Status; \
>      } \
>    } while (FALSE)
> -- 
> 2.32.0.windows.1
> 
> 
> 
> 
> 
> 

^ permalink raw reply	[flat|nested] 10+ messages in thread
* Re: [edk2-devel] [Patch 2/6] ArmPkg: Reproduce builds across source format changes
  2021-11-01 18:29 ` [Patch 2/6] ArmPkg: " Michael D Kinney
@ 2021-11-01 18:53   ` Leif Lindholm
  0 siblings, 0 replies; 10+ messages in thread
From: Leif Lindholm @ 2021-11-01 18:53 UTC (permalink / raw)
  To: devel, michael.d.kinney; +Cc: Ard Biesheuvel, Michael Kubacki

On Mon, Nov 01, 2021 at 11:29:38 -0700, Michael D Kinney wrote:
> REF: https://bugzilla.tianocore.org/show_bug.cgi?id=3688
> 
> Use DEBUG_LINE_NUMBER instead of __LINE__.
> 
> Cc: Leif Lindholm <leif@nuviainc.com>
> Cc: Ard Biesheuvel <ardb+tianocore@kernel.org>
> Cc: Michael Kubacki <michael.kubacki@microsoft.com>
> Signed-off-by: Michael D Kinney <michael.d.kinney@intel.com>

Reviewed-by: Leif Lindholm <leif@nuviainc.com>

> ---
>  .../Universal/Smbios/ProcessorSubClassDxe/ProcessorSubClass.c   | 2 +-
>  .../Smbios/SmbiosMiscDxe/Type00/MiscBiosVendorFunction.c        | 2 +-
>  .../SmbiosMiscDxe/Type01/MiscSystemManufacturerFunction.c       | 2 +-
>  .../SmbiosMiscDxe/Type02/MiscBaseBoardManufacturerFunction.c    | 2 +-
>  .../SmbiosMiscDxe/Type03/MiscChassisManufacturerFunction.c      | 2 +-
>  .../Type13/MiscNumberOfInstallableLanguagesFunction.c           | 2 +-
>  .../Smbios/SmbiosMiscDxe/Type32/MiscBootInformationFunction.c   | 2 +-
>  7 files changed, 7 insertions(+), 7 deletions(-)
> 
> diff --git a/ArmPkg/Universal/Smbios/ProcessorSubClassDxe/ProcessorSubClass.c b/ArmPkg/Universal/Smbios/ProcessorSubClassDxe/ProcessorSubClass.c
> index 4b409ff7458c..cbdf6df01ee0 100644
> --- a/ArmPkg/Universal/Smbios/ProcessorSubClassDxe/ProcessorSubClass.c
> +++ b/ArmPkg/Universal/Smbios/ProcessorSubClassDxe/ProcessorSubClass.c
> @@ -689,7 +689,7 @@ AddSmbiosProcessorTypeTable (
>  
>    if (EFI_ERROR (Status)) {
>      DEBUG ((DEBUG_ERROR, "[%a]:[%dL] Smbios Type04 Table Log Failed! %r \n",
> -            __FUNCTION__, __LINE__, Status));
> +            __FUNCTION__, DEBUG_LINE_NUMBER, Status));
>    }
>    FreePool (Type4Record);
>  
> diff --git a/ArmPkg/Universal/Smbios/SmbiosMiscDxe/Type00/MiscBiosVendorFunction.c b/ArmPkg/Universal/Smbios/SmbiosMiscDxe/Type00/MiscBiosVendorFunction.c
> index 5679ebaac8a5..2506c03d64b1 100644
> --- a/ArmPkg/Universal/Smbios/SmbiosMiscDxe/Type00/MiscBiosVendorFunction.c
> +++ b/ArmPkg/Universal/Smbios/SmbiosMiscDxe/Type00/MiscBiosVendorFunction.c
> @@ -271,7 +271,7 @@ SMBIOS_MISC_TABLE_FUNCTION (MiscBiosVendor)
>    Status = SmbiosMiscAddRecord ((UINT8*)SmbiosRecord, NULL);
>    if (EFI_ERROR (Status)) {
>      DEBUG ((DEBUG_ERROR, "[%a]:[%dL] Smbios Type00 Table Log Failed! %r \n",
> -           __FUNCTION__, __LINE__, Status));
> +           __FUNCTION__, DEBUG_LINE_NUMBER, Status));
>    }
>  
>    FreePool (SmbiosRecord);
> diff --git a/ArmPkg/Universal/Smbios/SmbiosMiscDxe/Type01/MiscSystemManufacturerFunction.c b/ArmPkg/Universal/Smbios/SmbiosMiscDxe/Type01/MiscSystemManufacturerFunction.c
> index 2c69c2593f5d..555557a2a946 100644
> --- a/ArmPkg/Universal/Smbios/SmbiosMiscDxe/Type01/MiscSystemManufacturerFunction.c
> +++ b/ArmPkg/Universal/Smbios/SmbiosMiscDxe/Type01/MiscSystemManufacturerFunction.c
> @@ -162,7 +162,7 @@ SMBIOS_MISC_TABLE_FUNCTION(MiscSystemManufacturer)
>    Status = SmbiosMiscAddRecord ((UINT8*)SmbiosRecord, NULL);
>    if (EFI_ERROR (Status)) {
>      DEBUG ((DEBUG_ERROR, "[%a]:[%dL] Smbios Type01 Table Log Failed! %r \n",
> -            __FUNCTION__, __LINE__, Status));
> +            __FUNCTION__, DEBUG_LINE_NUMBER, Status));
>    }
>  
>    FreePool (SmbiosRecord);
> diff --git a/ArmPkg/Universal/Smbios/SmbiosMiscDxe/Type02/MiscBaseBoardManufacturerFunction.c b/ArmPkg/Universal/Smbios/SmbiosMiscDxe/Type02/MiscBaseBoardManufacturerFunction.c
> index 097777a23904..f11295b199b3 100644
> --- a/ArmPkg/Universal/Smbios/SmbiosMiscDxe/Type02/MiscBaseBoardManufacturerFunction.c
> +++ b/ArmPkg/Universal/Smbios/SmbiosMiscDxe/Type02/MiscBaseBoardManufacturerFunction.c
> @@ -196,7 +196,7 @@ SMBIOS_MISC_TABLE_FUNCTION(MiscBaseBoardManufacturer)
>    Status = SmbiosMiscAddRecord ((UINT8 *)SmbiosRecord, NULL);
>    if (EFI_ERROR (Status)) {
>      DEBUG ((DEBUG_ERROR, "[%a]:[%dL] Smbios Type02 Table Log Failed! %r \n",
> -            __FUNCTION__, __LINE__, Status));
> +            __FUNCTION__, DEBUG_LINE_NUMBER, Status));
>    }
>  
>    FreePool (SmbiosRecord);
> diff --git a/ArmPkg/Universal/Smbios/SmbiosMiscDxe/Type03/MiscChassisManufacturerFunction.c b/ArmPkg/Universal/Smbios/SmbiosMiscDxe/Type03/MiscChassisManufacturerFunction.c
> index 66e3e5327fc3..d64046182b96 100644
> --- a/ArmPkg/Universal/Smbios/SmbiosMiscDxe/Type03/MiscChassisManufacturerFunction.c
> +++ b/ArmPkg/Universal/Smbios/SmbiosMiscDxe/Type03/MiscChassisManufacturerFunction.c
> @@ -178,7 +178,7 @@ SMBIOS_MISC_TABLE_FUNCTION(MiscChassisManufacturer)
>    Status = SmbiosMiscAddRecord ((UINT8*)SmbiosRecord, NULL);
>    if (EFI_ERROR (Status)) {
>      DEBUG ((DEBUG_ERROR, "[%a]:[%dL] Smbios Type03 Table Log Failed! %r \n",
> -            __FUNCTION__, __LINE__, Status));
> +            __FUNCTION__, DEBUG_LINE_NUMBER, Status));
>    }
>  
>    FreePool (SmbiosRecord);
> diff --git a/ArmPkg/Universal/Smbios/SmbiosMiscDxe/Type13/MiscNumberOfInstallableLanguagesFunction.c b/ArmPkg/Universal/Smbios/SmbiosMiscDxe/Type13/MiscNumberOfInstallableLanguagesFunction.c
> index 7c941b5c0709..017b410a16d0 100644
> --- a/ArmPkg/Universal/Smbios/SmbiosMiscDxe/Type13/MiscNumberOfInstallableLanguagesFunction.c
> +++ b/ArmPkg/Universal/Smbios/SmbiosMiscDxe/Type13/MiscNumberOfInstallableLanguagesFunction.c
> @@ -158,7 +158,7 @@ SMBIOS_MISC_TABLE_FUNCTION(MiscNumberOfInstallableLanguages)
>    Status = SmbiosMiscAddRecord ((UINT8*)SmbiosRecord, NULL);
>    if (EFI_ERROR (Status)) {
>      DEBUG ((DEBUG_ERROR, "[%a]:[%dL] Smbios Type13 Table Log Failed! %r \n",
> -            __FUNCTION__, __LINE__, Status));
> +            __FUNCTION__, DEBUG_LINE_NUMBER, Status));
>    }
>  
>    FreePool (SmbiosRecord);
> diff --git a/ArmPkg/Universal/Smbios/SmbiosMiscDxe/Type32/MiscBootInformationFunction.c b/ArmPkg/Universal/Smbios/SmbiosMiscDxe/Type32/MiscBootInformationFunction.c
> index 4be1e1cd29a9..c4ce6a5e7608 100644
> --- a/ArmPkg/Universal/Smbios/SmbiosMiscDxe/Type32/MiscBootInformationFunction.c
> +++ b/ArmPkg/Universal/Smbios/SmbiosMiscDxe/Type32/MiscBootInformationFunction.c
> @@ -68,7 +68,7 @@ SMBIOS_MISC_TABLE_FUNCTION(MiscBootInformation)
>    Status = SmbiosMiscAddRecord ((UINT8*)SmbiosRecord, NULL);
>    if (EFI_ERROR (Status)) {
>      DEBUG ((DEBUG_ERROR, "[%a]:[%dL] Smbios Type32 Table Log Failed! %r \n",
> -            __FUNCTION__, __LINE__, Status));
> +            __FUNCTION__, DEBUG_LINE_NUMBER, Status));
>    }
>  
>    FreePool (SmbiosRecord);
> -- 
> 2.32.0.windows.1
> 
> 
> 
> 
> 
> 

^ permalink raw reply	[flat|nested] 10+ messages in thread
* Re: [edk2-devel] [Patch 0/6] Reproduce builds across source format changes
  2021-11-01 18:29 [Patch 0/6] Reproduce builds across source format changes Michael D Kinney
                   ` (5 preceding siblings ...)
  2021-11-01 18:29 ` [Patch 6/6] OvmfPkg: " Michael D Kinney
@ 2021-11-01 19:03 ` Michael Kubacki
  6 siblings, 0 replies; 10+ messages in thread
From: Michael Kubacki @ 2021-11-01 19:03 UTC (permalink / raw)
  To: devel, michael.d.kinney

For the series:

Tested-by: Michael Kubacki <michael.kubacki@microsoft.com>

On 11/1/2021 2:29 PM, Michael D Kinney wrote:
> REF: https://bugzilla.tianocore.org/show_bug.cgi?id=3688
> 
> This change is required to help verify that source code formatting
> changes such as the use of uncrustify and line ending corrections
> do not have any functional differences. Source format changes may
> add or remove line endings that change the source file line numbers
> of C statements or may change the use of spaces in C expressions
> used in an ASSERT() statements. These types of changes can impact
> the generated binaries when DEBUG() and ASSERT() macros are
> enabled. The following set of changes adds 2 defines that can be used
> to override the use of __LINE__ in DEBUG() macros and the use of
> #Expression in ASSERT() macros.
> 
> * Add DEBUG_LINE_NUMBER define to DebugLib.h that is
>    by default mapped to __LINE__. A build can pre-define
>    DEBUG_LINE_NUMBER to use a fixed value.
> * Add DEBUG_EXPRESSION_STRING(Expression) macros to
>    DebugLib.h that is by default mapped to #Expression.
>    A build can define DEBUG_EXPRESSION_STRING_VALUE to
>    set all expression strings to a fixed string value.
> * Use DEBUG_LINE_NUMBER instead of __LINE__.
> * Use DEBUG_EXPRESSION_STRING instead of #Expression.
> 
> Submodules that use __LINE__ are not updated. These do not
> currently impact build reproducibility unless the debug features
> of those submodules are enabled.
> 
> The one exception is the UnitTestFrameworkPkg cmocka submodule
> that uses `__LINE__`.  This means that the binaries generated by host
> based unit tests that use cmocka features may not be identical across
> a source format change.
> 
> 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: Michael Kubacki michael.kubacki@microsoft.com
> Cc: Jian J Wang jian.j.wang@intel.com
> Cc: Maciej Rabeda maciej.rabeda@linux.intel.com
> Cc: Jiaxin Wu jiaxin.wu@intel.com
> Cc: Siyuan Fu siyuan.fu@intel.com
> Cc: Liming Gao gaoliming@byosoft.com.cn
> Cc: Leif Lindholm leif@nuviainc.com
> Cc: Zhiguang Liu zhiguang.liu@intel.com
> Signed-off-by: Michael D Kinney michael.d.kinney@intel.com
> 
> Michael D Kinney (6):
>    MdePkg: Reproduce builds across source format changes
>    ArmPkg: Reproduce builds across source format changes
>    MdeModulePkg: Reproduce builds across source format changes
>    NetworkPkg: Reproduce builds across source format changes
>    SecurityPkg: Reproduce builds across source format changes
>    OvmfPkg: Reproduce builds across source format changes
> 
>   .../ProcessorSubClassDxe/ProcessorSubClass.c  |  2 +-
>   .../Type00/MiscBiosVendorFunction.c           |  2 +-
>   .../Type01/MiscSystemManufacturerFunction.c   |  2 +-
>   .../MiscBaseBoardManufacturerFunction.c       |  2 +-
>   .../Type03/MiscChassisManufacturerFunction.c  |  2 +-
>   ...MiscNumberOfInstallableLanguagesFunction.c |  2 +-
>   .../Type32/MiscBootInformationFunction.c      |  2 +-
>   .../Bus/Pci/PciHostBridgeDxe/PciHostBridge.c  |  2 +-
>   .../UefiBootManagerLib/BmDriverHealth.c       |  2 +-
>   MdePkg/Include/Library/DebugLib.h             | 42 +++++++++++++++++--
>   MdePkg/Include/Library/UnitTestLib.h          | 18 ++++----
>   MdePkg/Library/BaseLib/SafeString.c           |  2 +-
>   NetworkPkg/Include/Library/NetLib.h           |  8 ++--
>   NetworkPkg/Library/DxeNetLib/DxeNetLib.c      |  2 +-
>   OvmfPkg/Csm/LegacyBiosDxe/LegacyPci.c         |  6 +--
>   .../PlatformBootManagerLib/BdsPlatform.c      |  4 +-
>   .../PlatformBootManagerLibBhyve/BdsPlatform.c |  2 +-
>   .../PlatformBootManagerLibGrub/BdsPlatform.c  |  4 +-
>   .../Include/Library/TcgStorageCoreLib.h       |  4 +-
>   19 files changed, 72 insertions(+), 38 deletions(-)
> 

^ permalink raw reply	[flat|nested] 10+ messages in thread
end of thread, other threads:[~2021-11-01 19:03 UTC | newest]

Thread overview: 10+ messages (download: mbox.gz follow: Atom feed
-- links below jump to the message on this page --
2021-11-01 18:29 [Patch 0/6] Reproduce builds across source format changes Michael D Kinney
2021-11-01 18:29 ` [Patch 1/6] MdePkg: " Michael D Kinney
2021-11-01 18:52   ` [edk2-devel] " Leif Lindholm
2021-11-01 18:29 ` [Patch 2/6] ArmPkg: " Michael D Kinney
2021-11-01 18:53   ` [edk2-devel] " Leif Lindholm
2021-11-01 18:29 ` [Patch 3/6] MdeModulePkg: " Michael D Kinney
2021-11-01 18:29 ` [Patch 4/6] NetworkPkg: " Michael D Kinney
2021-11-01 18:29 ` [Patch 5/6] SecurityPkg: " Michael D Kinney
2021-11-01 18:29 ` [Patch 6/6] OvmfPkg: " Michael D Kinney
2021-11-01 19:03 ` [edk2-devel] [Patch 0/6] " Michael Kubacki

This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox