Re: [PATCH 1/1] OvmfPkg/IoMmuDxe: shut up "unused-const-variable" gcc-6 warning in RELEASE

["Gao, Liming" <liming.gao@intel.com>]

Laszlo:
  I add -Wno-unused-const-variable option in GCC5 RELEASE option, and build OvmfPkg with GCC5.3. I don't meet any warning or error. Do you meet any issue? So, you think we can't add this option in GCC5 tool chain. 

Thanks
Liming
> -----Original Message-----
> From: edk2-devel [mailto:edk2-devel-bounces@lists.01.org] On Behalf Of Laszlo Ersek
> Sent: Thursday, September 7, 2017 12:48 AM
> To: edk2-devel-01 <edk2-devel@lists.01.org>
> Cc: Justen, Jordan L <jordan.l.justen@intel.com>; Brijesh Singh <brijesh.singh@amd.com>; Thomas Lamprecht
> <t.lamprecht@proxmox.com>; Ard Biesheuvel <ard.biesheuvel@linaro.org>
> Subject: [edk2] [PATCH 1/1] OvmfPkg/IoMmuDxe: shut up "unused-const-variable" gcc-6 warning in RELEASE
> 
> Starting with gcc-6, a new warning option called "-Wunused-const-variable"
> has become available (and enabled by default under our build settings):
> 
>   https://gcc.gnu.org/onlinedocs/gcc-6.4.0/gcc/Warning-Options.html
> 
> We should give it the same treatment as Ard gave "unused-but-set-variable"
> in commit 20d00edf21d2 ("BaseTools/GCC: set -Wno-unused-but-set-variables
> only on RELEASE builds", 2016-03-24); i.e., we should restrict the warning
> to the DEBUG build target.
> 
> However, because the new warning is gcc-6+ only, we cannot add
> "-Wno-unused-const-variable" to any GCC5 macros in
> "BaseTools/Conf/tools_def.template". While the GCC6 toolchain and/or the
> desired handling of the new warning are investigated in
> <https://bugzilla.tianocore.org/show_bug.cgi?id=700>, suppress the warning
> for gcc-6+ (in RELEASE builds) as follows:
> 
> - Replace the "mBusMasterOperationName" array with the
>   BUS_MASTER_OPERATION_NAME(Expression, ShortName) macro that compares
>   Expression against EdkiiIoMmuOperationBusMaster<ShortName>, and in case
>   of a match, evaluates to "ShortName", stringified,
> 
> - when composing the DEBUG message -- which the preprocessor might
>   eliminate from RELEASE builds, thereby removing its references to
>   variables --, build a ladder of comparisons with
>   BUS_MASTER_OPERATION_NAME().
> 
> Cc: Ard Biesheuvel <ard.biesheuvel@linaro.org>
> Cc: Brijesh Singh <brijesh.singh@amd.com>
> Cc: Jordan Justen <jordan.l.justen@intel.com>
> Cc: Thomas Lamprecht <t.lamprecht@proxmox.com>
> Reported-by: Thomas Lamprecht <t.lamprecht@proxmox.com>
> Contributed-under: TianoCore Contribution Agreement 1.1
> Signed-off-by: Laszlo Ersek <lersek@redhat.com>
> ---
>  OvmfPkg/IoMmuDxe/AmdSevIoMmu.c | 29 +++++++++++---------
>  1 file changed, 16 insertions(+), 13 deletions(-)
> 
> diff --git a/OvmfPkg/IoMmuDxe/AmdSevIoMmu.c b/OvmfPkg/IoMmuDxe/AmdSevIoMmu.c
> index bc57de5b572b..dcd03d8f0474 100644
> --- a/OvmfPkg/IoMmuDxe/AmdSevIoMmu.c
> +++ b/OvmfPkg/IoMmuDxe/AmdSevIoMmu.c
> @@ -45,17 +45,17 @@ STATIC LIST_ENTRY mRecycledMapInfos = INITIALIZE_LIST_HEAD_VARIABLE (
>  #define COMMON_BUFFER_SIG SIGNATURE_64 ('C', 'M', 'N', 'B', 'U', 'F', 'F', 'R')
> 
>  //
> -// ASCII names for EDKII_IOMMU_OPERATION constants, for debug logging.
> +// The following macro builds the first two operands of a conditional (ternary)
> +// operator that matches Expression against the EDKII_IOMMU_OPERATION constant
> +// derived from ShortName. In case of a match, the replacement text evaluates
> +// to an ASCII string literal that stands for the constant. The replacement
> +// text stops before the colon (":"). The macro invocation site is supposed to
> +// provide the colon and the third operand, either as another invocation of the
> +// same macro, or as a default (fallback) string literal.
>  //
> -STATIC CONST CHAR8 * CONST
> -mBusMasterOperationName[EdkiiIoMmuOperationMaximum] = {
> -  "Read",
> -  "Write",
> -  "CommonBuffer",
> -  "Read64",
> -  "Write64",
> -  "CommonBuffer64"
> -};
> +#define BUS_MASTER_OPERATION_NAME(Expression, ShortName) \
> +          ((Expression) == (EdkiiIoMmuOperationBusMaster ## ShortName)) ? \
> +          (# ShortName)
> 
>  //
>  // The following structure enables Map() and Unmap() to perform in-place
> @@ -133,9 +133,12 @@ IoMmuMap (
>      DEBUG_VERBOSE,
>      "%a: Operation=%a Host=0x%p Bytes=0x%Lx\n",
>      __FUNCTION__,
> -    ((Operation >= 0 &&
> -      Operation < ARRAY_SIZE (mBusMasterOperationName)) ?
> -     mBusMasterOperationName[Operation] :
> +    (BUS_MASTER_OPERATION_NAME (Operation, Read) :
> +     BUS_MASTER_OPERATION_NAME (Operation, Write) :
> +     BUS_MASTER_OPERATION_NAME (Operation, CommonBuffer) :
> +     BUS_MASTER_OPERATION_NAME (Operation, Read64) :
> +     BUS_MASTER_OPERATION_NAME (Operation, Write64) :
> +     BUS_MASTER_OPERATION_NAME (Operation, CommonBuffer64) :
>       "Invalid"),
>      HostAddress,
>      (UINT64)((NumberOfBytes == NULL) ? 0 : *NumberOfBytes)
> --
> 2.14.1.3.gb7cf6e02401b
> 
> _______________________________________________
> edk2-devel mailing list
> edk2-devel@lists.01.org
> https://lists.01.org/mailman/listinfo/edk2-devel