Vitaly, Thank you for the contribution.
There are a couple points about this approach that need to be discussed. You have included the <Library/DebugCommonLib.h> from MdePkg/Include/Library/DebugLib.h. It is very rare for a lib class to include another lib class. This means that a module that has a dependency on the DebugLib class inherits a hidden dependency on the DebugCommonLib class. For module INF files, we require the INF file to list all the lib classes that the module sources directly use. Since a module that uses the DebugLib uses the ASSERT() and DEBUG() macros, all the APIs that the ASSERT() and DEBUG() macros use are also directly used by the module. With this patch series, these macros now use the DebugCommonLib class APIs, which means any module that uses the DebugLib also directly uses the DebugCommonLib. The INF files for all modules that use the DebugLib should also be updated to list the DebugCommonLib. If we go down that path, then it would be cleaner for the modules to include both DebugLib.h and DebugCommonLib.h so the list of includes matches the list of lib classes in the INF file. This would be an even much larger change than the patch series already under review. I think there is general agreement that the ASSERT_CONSTRAINT() feature is valuable, and the sticking point has been the complexity of the change and the impact to the existing modules and platforms. In order to address the original problem statement and Bugzilla: https://bugzilla.tianocore.org/show_bug.cgi?id=2054 Perhaps we should go back to the original proposal that adds one new PCD so the string APIs in the BaseLib can filter out ASSERT() conditions for UEFI Applications that want return status behavior without ASSERT() behavior. The work on this patch series and the other proposals have all been very valuable and we can continue to look for ways to implement the general purpose ASSERT_CONSTRAINT() without so many module and platform side effects. Thanks, Mike > -----Original Message----- > From: Vitaly Cheptsov <chept...@ispras.ru> > Sent: Monday, May 11, 2020 8:41 AM > To: devel@edk2.groups.io > Cc: Andrew Fish <af...@apple.com>; Kinney, Michael D > <michael.d.kin...@intel.com>; Marvin Häuser > <mhaeu...@outlook.de>; Gao, Liming > <liming....@intel.com>; Gao, Zhichao > <zhichao....@intel.com>; Laszlo Ersek > <ler...@redhat.com> > Subject: [PATCH V4 00/27] Disabling safe string > constraint assertions > > Cc: Andrew Fish <af...@apple.com> > Cc: Mike Kinney <michael.d.kin...@intel.com> > Cc: Marvin Häuser <mhaeu...@outlook.de> > Cc: Liming Gao <liming....@intel.com> > Cc: Zhichao Gao <zhichao....@intel.com> > CC: Laszlo Ersek <ler...@redhat.com> > > This changesest hopefully finally resolves the > longstanding > https://bugzilla.tianocore.org/show_bug.cgi?id=2054 > > Requesting to merge into edk2-stable202005. > > Vitaly Cheptsov (27): > MdePkg: Introduce DebugCommonLib interface and > BaseDebugCommonLib > UnitTestFrameworkPkg: Add support for DebugCommonLib > MdePkg: Add support for DebugCommonLib > MdeModulePkg: Add support for DebugCommonLib > ArmPkg: Add support for DebugCommonLib > ArmPlatformPkg: Add support for DebugCommonLib > ArmVirtPkg: Add support for DebugCommonLib > CryptoPkg: Add support for DebugCommonLib > DynamicTablesPkg: Add support for DebugCommonLib > EmbeddedPkg: Add support for DebugCommonLib > EmulatorPkg: Add support for DebugCommonLib > FatPkg: Add support for DebugCommonLib > FmpDevicePkg: Add support for DebugCommonLib > IntelFsp2Pkg: Add support for DebugCommonLib > IntelFsp2WrapperPkg: Add support for DebugCommonLib > OvmfPkg: Add support for DebugCommonLib > NetworkPkg: Add support for DebugCommonLib > ShellPkg: Add support for DebugCommonLib > SecurityPkg: Add support for DebugCommonLib > PcAtChipsetPkg: Add support for DebugCommonLib > SignedCapsulePkg: Add support for DebugCommonLib > SourceLevelDebugPkg: Add support for DebugCommonLib > StandaloneMmPkg: Add support for DebugCommonLib > UefiCpuPkg: Add support for DebugCommonLib > UefiPayloadPkg: Add support for DebugCommonLib > MdePkg: Introduce assertion on constraint debug mask > bit > MdePkg: Use assertion on constraint violation bit in > SafeString > > ArmPkg/ArmPkg.dsc > | 1 + > ArmPkg/Drivers/ArmCrashDumpDxe/ArmCrashDumpDxe.dsc > | 1 + > ArmPkg/Library/SemiHostingDebugLib/DebugLib.c > | 84 ---------- > > ArmPkg/Library/SemiHostingDebugLib/SemiHostingDebugLib. > inf | 3 +- > ArmPlatformPkg/ArmPlatformPkg.dsc > | 1 + > ArmVirtPkg/ArmVirt.dsc.inc > | 1 + > CryptoPkg/CryptoPkg.dsc > | 2 + > DynamicTablesPkg/DynamicTablesPkg.dsc > | 1 + > EmbeddedPkg/EmbeddedPkg.dsc > | 1 + > EmulatorPkg/EmulatorPkg.dsc > | 1 + > FatPkg/FatPkg.dsc > | 1 + > FmpDevicePkg/FmpDevicePkg.dsc > | 1 + > IntelFsp2Pkg/IntelFsp2Pkg.dsc > | 1 + > > IntelFsp2Pkg/Library/BaseFspDebugLibSerialPort/BaseFspD > ebugLibSerialPort.inf | 1 + > > IntelFsp2Pkg/Library/BaseFspDebugLibSerialPort/DebugLib > .c | 97 ----------- > IntelFsp2WrapperPkg/IntelFsp2WrapperPkg.dsc > | 1 + > MdeModulePkg/Library/PeiDebugLibDebugPpi/DebugLib.c > | 100 ------------ > > MdeModulePkg/Library/PeiDebugLibDebugPpi/PeiDebugLibDeb > ugPpi.inf | 1 + > > MdeModulePkg/Library/PeiDxeDebugLibReportStatusCode/Deb > ugLib.c | 98 ----------- > > MdeModulePkg/Library/PeiDxeDebugLibReportStatusCode/Pei > DxeDebugLibReportStatusCode.inf | 1 + > MdeModulePkg/MdeModulePkg.dsc > | 1 + > MdePkg/Include/Library/BaseLib.h > | 120 +++++++------- > MdePkg/Include/Library/DebugCommonLib.h > | 172 ++++++++++++++++++++ > MdePkg/Include/Library/DebugLib.h > | 155 +++--------------- > > MdePkg/Library/BaseDebugCommonLib/BaseDebugCommonLib.in > f | 39 +++++ > > MdePkg/Library/BaseDebugCommonLib/BaseDebugCommonLib.un > i | 16 ++ > MdePkg/Library/BaseDebugCommonLib/DebugCommonLib.c > | 133 +++++++++++++++ > MdePkg/Library/BaseDebugLibNull/BaseDebugLibNull.inf > | 10 ++ > MdePkg/Library/BaseDebugLibNull/DebugLib.c > | 98 ----------- > > MdePkg/Library/BaseDebugLibSerialPort/BaseDebugLibSeria > lPort.inf | 1 + > MdePkg/Library/BaseDebugLibSerialPort/DebugLib.c > | 98 ----------- > MdePkg/Library/BaseLib/BaseLib.inf > | 1 + > MdePkg/Library/BaseLib/SafeString.c > | 2 +- > MdePkg/Library/DxeRuntimeDebugLibSerialPort/DebugLib.c > | 98 ----------- > > MdePkg/Library/DxeRuntimeDebugLibSerialPort/DxeRuntimeD > ebugLibSerialPort.inf | 1 + > MdePkg/Library/UefiDebugLibConOut/DebugLib.c > | 98 ----------- > > MdePkg/Library/UefiDebugLibConOut/UefiDebugLibConOut.in > f | 1 + > > MdePkg/Library/UefiDebugLibDebugPortProtocol/DebugLib.c > | 99 ----------- > > MdePkg/Library/UefiDebugLibDebugPortProtocol/UefiDebugL > ibDebugPortProtocol.inf | 1 + > MdePkg/Library/UefiDebugLibStdErr/DebugLib.c > | 98 ----------- > > MdePkg/Library/UefiDebugLibStdErr/UefiDebugLibStdErr.in > f | 1 + > MdePkg/MdePkg.dec > | 6 +- > MdePkg/MdePkg.dsc > | 1 + > MdePkg/MdePkg.uni > | 3 +- > NetworkPkg/NetworkPkg.dsc > | 1 + > OvmfPkg/Library/PlatformDebugLibIoPort/DebugLib.c > | 98 ----------- > > OvmfPkg/Library/PlatformDebugLibIoPort/PlatformRomDebug > LibIoPort.inf | 1 + > OvmfPkg/OvmfPkgIa32.dsc > | 1 + > OvmfPkg/OvmfPkgIa32X64.dsc > | 1 + > OvmfPkg/OvmfPkgX64.dsc > | 1 + > OvmfPkg/OvmfXen.dsc > | 1 + > PcAtChipsetPkg/PcAtChipsetPkg.dsc > | 1 + > SecurityPkg/SecurityPkg.dsc > | 1 + > ShellPkg/ShellPkg.dsc > | 1 + > SignedCapsulePkg/SignedCapsulePkg.dsc > | 1 + > SourceLevelDebugPkg/SourceLevelDebugPkg.dsc > | 1 + > StandaloneMmPkg/StandaloneMmPkg.dsc > | 1 + > UefiCpuPkg/UefiCpuPkg.dsc > | 1 + > UefiPayloadPkg/UefiPayloadPkgIa32.dsc > | 1 + > UefiPayloadPkg/UefiPayloadPkgIa32X64.dsc > | 1 + > > UnitTestFrameworkPkg/Library/Posix/DebugLibPosix/DebugL > ibPosix.c | 94 ----------- > > UnitTestFrameworkPkg/Library/Posix/DebugLibPosix/DebugL > ibPosix.inf | 1 + > UnitTestFrameworkPkg/UnitTestFrameworkPkgHost.dsc.inc > | 1 + > > UnitTestFrameworkPkg/UnitTestFrameworkPkgTarget.dsc.inc > | 1 + > 64 files changed, 501 insertions(+), 1360 deletions(-) > create mode 100644 > MdePkg/Include/Library/DebugCommonLib.h > create mode 100644 > MdePkg/Library/BaseDebugCommonLib/BaseDebugCommonLib.in > f > create mode 100644 > MdePkg/Library/BaseDebugCommonLib/BaseDebugCommonLib.un > i > create mode 100644 > MdePkg/Library/BaseDebugCommonLib/DebugCommonLib.c > > -- > 2.24.2 (Apple Git-127) -=-=-=-=-=-=-=-=-=-=-=- Groups.io Links: You receive all messages sent to this group. View/Reply Online (#59220): https://edk2.groups.io/g/devel/message/59220 Mute This Topic: https://groups.io/mt/74138532/21656 Group Owner: devel+ow...@edk2.groups.io Unsubscribe: https://edk2.groups.io/g/devel/unsub [arch...@mail-archive.com] -=-=-=-=-=-=-=-=-=-=-=-
