On 05/30/16 15:22, Marvin Häuser wrote: > When the only point of use of a variable is a DEBUG() or ASSERT() > expression, most compilers will issue a warning when targeting > RELEASE. Cast the parameters to VOID to silence these warnings. > > Contributed-under: TianoCore Contribution Agreement 1.0 > Signed-off-by: Marvin Haeuser <[email protected]> > --- > MdePkg/Include/Library/DebugLib.h | 19 ++++++++++++++----- > 1 file changed, 14 insertions(+), 5 deletions(-)
This seems incorrect to me. The patch doesn't only cast existing expressions to VOID, it evaluates expressions for one more time. If the expressions have side effects (which may or may not be a smart thing, but it depends on the caller of DEBUG()), this patch will cause problems. Instead, the compiler flags should be changed for RELEASE builds not to warn about such variables. Please see the following messages: http://thread.gmane.org/gmane.comp.bios.edk2.devel/9184/focus=9454 http://thread.gmane.org/gmane.comp.bios.edk2.devel/9702/focus=9735 In brief, we converted the tree to be "clean" with regard to gcc's "-Wunused-but-set-variable", but only for DEBUG builds. Thanks Laszlo > diff --git a/MdePkg/Include/Library/DebugLib.h > b/MdePkg/Include/Library/DebugLib.h > index 93b6f8df34ae..25c8ebb95740 100644 > --- a/MdePkg/Include/Library/DebugLib.h > +++ b/MdePkg/Include/Library/DebugLib.h > @@ -270,7 +270,7 @@ DebugPrintLevelEnabled ( > } while (FALSE) > #define _DEBUG(Expression) _DEBUG_PRINT Expression > #else > -#define _DEBUG(Expression) DebugPrint Expression > + #define _DEBUG(Expression) DebugPrint Expression > #endif > > /** > @@ -288,6 +288,8 @@ DebugPrintLevelEnabled ( > #if !defined(MDEPKG_NDEBUG) > #define ASSERT(Expression) \ > do { \ > + (VOID)(Expression); \ > + \ > if (DebugAssertEnabled ()) { \ > if (!(Expression)) { \ > _ASSERT (Expression); \ > @@ -295,7 +297,7 @@ DebugPrintLevelEnabled ( > } \ > } while (FALSE) > #else > - #define ASSERT(Expression) > + #define ASSERT(Expression) (VOID)(Expression) > #endif > > /** > @@ -312,13 +314,15 @@ DebugPrintLevelEnabled ( > **/ > #if !defined(MDEPKG_NDEBUG) > #define DEBUG(Expression) \ > + (VOID)(Expression); \ > + \ > do { \ > if (DebugPrintEnabled ()) { \ > _DEBUG (Expression); \ > } \ > } while (FALSE) > #else > - #define DEBUG(Expression) > + #define DEBUG(Expression) (VOID)(Expression) > #endif > > /** > @@ -336,6 +340,8 @@ DebugPrintLevelEnabled ( > #if !defined(MDEPKG_NDEBUG) > #define ASSERT_EFI_ERROR(StatusParameter) > \ > do { > \ > + (VOID)(StatusParameter); > \ > + > \ > if (DebugAssertEnabled ()) { > \ > if (EFI_ERROR (StatusParameter)) { > \ > DEBUG ((EFI_D_ERROR, "\nASSERT_EFI_ERROR (Status = %r)\n", > StatusParameter)); \ > @@ -344,7 +350,7 @@ DebugPrintLevelEnabled ( > } > \ > } while (FALSE) > #else > - #define ASSERT_EFI_ERROR(StatusParameter) > + #define ASSERT_EFI_ERROR(StatusParameter) (VOID)(StatusParameter) > #endif > > /** > @@ -372,6 +378,9 @@ DebugPrintLevelEnabled ( > #if !defined(MDEPKG_NDEBUG) > #define ASSERT_PROTOCOL_ALREADY_INSTALLED(Handle, Guid) > \ > do { > \ > + (VOID)(Handle); > \ > + (VOID)(Guid); > \ > + > \ > if (DebugAssertEnabled ()) { > \ > VOID *Instance; > \ > ASSERT (Guid != NULL); > \ > @@ -387,7 +396,7 @@ DebugPrintLevelEnabled ( > } > \ > } while (FALSE) > #else > - #define ASSERT_PROTOCOL_ALREADY_INSTALLED(Handle, Guid) > + #define ASSERT_PROTOCOL_ALREADY_INSTALLED(Handle, Guid) (VOID)(Handle) > #endif > > /** > _______________________________________________ edk2-devel mailing list [email protected] https://lists.01.org/mailman/listinfo/edk2-devel
