Hey Laszlo, Thanks for your comment. To be honest, I was not aware of any side effects when casting to VOID - that's what I have been used to in the past and also saw it in several sources, including Microsoft (yes, not a good example for good src :) ). But as the issue is obviously known already, which I didn't know either, I will just wait it out. Though, if only DEBUG builds are checked for unused variables, might be an issue for those who only build RELEASE.
Regards, Marvin. > -----Original Message----- > From: Laszlo Ersek [mailto:ler...@redhat.com] > Sent: Monday, May 30, 2016 6:03 PM > To: Marvin Häuser <marvin.haeu...@outlook.com>; edk2- > de...@lists.01.org <edk2-de...@ml01.01.org> > Cc: michael.d.kin...@intel.com; liming....@intel.com; Ard Biesheuvel > <ard.biesheu...@linaro.org> > Subject: Re: [edk2] [PATCH v1 1/1] MdePkg/DebugLib: Ignore DEBUG > variables in RELEASE builds. > > 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 <marvin.haeu...@outlook.com> > > --- > > 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 edk2-devel@lists.01.org https://lists.01.org/mailman/listinfo/edk2-devel
