On 7 December 2017 at 15:12, Ard Biesheuvel <[email protected]> wrote: > ASSERT_EFI_ERROR () is currently defined as > > #if !defined(MDEPKG_NDEBUG) > #define ASSERT_EFI_ERROR(StatusParameter) > \ > do { > \ > if (DebugAssertEnabled ()) { > \ > if (EFI_ERROR (StatusParameter)) { > \ > DEBUG ((EFI_D_ERROR, "\nASSERT_EFI_ERROR (Status = %r)\n", > StatusParameter)); \ > _ASSERT (!EFI_ERROR (StatusParameter)); > \ > } > \ > } > \ > } while (FALSE) > #else > #define ASSERT_EFI_ERROR(StatusParameter) > #endif > > This is suboptimal, given that the DebugAssertEnabled () call in the > outer if must be executed unconditionally, since the compiler does not > know that it does not have any side effects. Instead, let's swap the > two ifs, and only call DebugAssertEnabled () if StatusParameter contains > an error value to begin with. Do the same for ASSERT_RETURN_ERROR > as well. >
I just noticed we could do the same for ASSERT () as well. > Contributed-under: TianoCore Contribution Agreement 1.1 > Signed-off-by: Ard Biesheuvel <[email protected]> > Reported-by: Alexei Fedorov <[email protected]> > --- > MdePkg/Include/Library/DebugLib.h | 8 ++++---- > 1 file changed, 4 insertions(+), 4 deletions(-) > > diff --git a/MdePkg/Include/Library/DebugLib.h > b/MdePkg/Include/Library/DebugLib.h > index 3a910e6a208b..8369c378e79c 100644 > --- a/MdePkg/Include/Library/DebugLib.h > +++ b/MdePkg/Include/Library/DebugLib.h > @@ -337,8 +337,8 @@ DebugPrintLevelEnabled ( > #if !defined(MDEPKG_NDEBUG) > #define ASSERT_EFI_ERROR(StatusParameter) > \ > do { > \ > - if (DebugAssertEnabled ()) { > \ > - if (EFI_ERROR (StatusParameter)) { > \ > + if (EFI_ERROR (StatusParameter)) { > \ > + if (DebugAssertEnabled ()) { > \ > DEBUG ((EFI_D_ERROR, "\nASSERT_EFI_ERROR (Status = %r)\n", > StatusParameter)); \ > _ASSERT (!EFI_ERROR (StatusParameter)); > \ > } > \ > @@ -363,8 +363,8 @@ DebugPrintLevelEnabled ( > #if !defined(MDEPKG_NDEBUG) > #define ASSERT_RETURN_ERROR(StatusParameter) \ > do { \ > - if (DebugAssertEnabled ()) { \ > - if (RETURN_ERROR (StatusParameter)) { \ > + if (RETURN_ERROR (StatusParameter)) { \ > + if (DebugAssertEnabled ()) { \ > DEBUG ((DEBUG_ERROR, "\nASSERT_RETURN_ERROR (Status = %r)\n", \ > StatusParameter)); \ > _ASSERT (!RETURN_ERROR (StatusParameter)); \ > -- > 2.11.0 > _______________________________________________ edk2-devel mailing list [email protected] https://lists.01.org/mailman/listinfo/edk2-devel
