On 24 March 2016 at 11:36, Laszlo Ersek <ler...@redhat.com> wrote:
> This series removes unused but set variables from edk2, so that the
> source builds after Ard's patch (the last one in this series) lets
> -Wunused-but-set-variables take effect for GCC DEBUG builds.
>
> I used the following method for locating these warnings:
> * "-Werror" was temporarily removed;
> * I rounded up the DSC files in the tree;
> * I built each DSC file for:
> { RELEASE, DEBUG } x { IA32, X64, ARM, AARCH64 };
> * some platform DSCs obviously don't build for all four arches above, I
> ignored those errors;
> * some platforms I couldn't build at all, I ignored those too;
> * for the rest (i.e., when a given DSC built for at least some
> platforms), I collected the warnings from the logs, across all the
> arches that the DSC did build for, and I silenced them.
>
> Maintainers are invited to exert caution while reviewing the patches.
> Namely, in a few cases, the silencing is almost certainly wrong, and
> should be replaced with proper error checking. These are the cases when
> a Status variable is set from a function call, but then never checked. I
> didn't try to handle these errors myself; I just removed the Status
> variable.
>
I suppose it would generally be more correct to add a
ASSERT_EFI_ERROR(Status) rather than remove Status entirely. However,
this may not be universally true, so it is up to the respective
maintainers to decide whether a) the function call is assumed to
succeed or b) the function call may fail but we don't care. The ASSERT
only covers a)
> Obviously, this method does not change behavior, but the gcc warning in
> this case signals something much worse than a forgotten variable: lack
> of error handling. So, dear maintainers, if you recognize this pattern
> on your turf, please indicate in your reviews that you are going to post
> a real error handling fix for that warning.
>
> The list of DSC's I could build (at least for some architectures):
>
> AppPkg/AppPkg.dsc
> ArmPkg/ArmPkg.dsc
> ArmVirtPkg/ArmVirtQemu.dsc
> ArmVirtPkg/ArmVirtQemuKernel.dsc
> ArmVirtPkg/ArmVirtXen.dsc
> BeagleBoardPkg/BeagleBoardPkg.dsc
> CorebootPayloadPkg/CorebootPayloadPkgIa32.dsc
> CorebootPayloadPkg/CorebootPayloadPkgIa32X64.dsc
> CryptoPkg/CryptoPkg.dsc
> DuetPkg/DuetPkgIa32.dsc
> DuetPkg/DuetPkgX64.dsc
> EdkCompatibilityPkg/EdkCompatibilityPkg.dsc
> EmulatorPkg/EmulatorPkg.dsc
> IntelFrameworkModulePkg/IntelFrameworkModulePkg.dsc
> IntelFrameworkPkg/IntelFrameworkPkg.dsc
> IntelFspPkg/IntelFspPkg.dsc
> IntelFspWrapperPkg/IntelFspWrapperPkg.dsc
> MdeModulePkg/MdeModulePkg.dsc
> MdePkg/MdePkg.dsc
> NetworkPkg/NetworkPkg.dsc
> OptionRomPkg/OptionRomPkg.dsc
> OvmfPkg/OvmfPkgIa32.dsc
> OvmfPkg/OvmfPkgIa32X64.dsc
> OvmfPkg/OvmfPkgX64.dsc
> PcAtChipsetPkg/PcAtChipsetPkg.dsc
> PerformancePkg/PerformancePkg.dsc
> QuarkSocPkg/QuarkSocPkg.dsc
> SecurityPkg/SecurityPkg.dsc
> ShellPkg/ShellPkg.dsc
> SourceLevelDebugPkg/SourceLevelDebugPkg.dsc
> StdLib/StdLib.dsc
> UefiCpuPkg/UefiCpuPkg.dsc
>
> (Side note: CryptoPkg implies OpenSSL-1.0.2g.)
>
> The list of DSC's that wouldn't build for me at all (on Linux, with
> GCC48):
>
> EmbeddedPkg/EmbeddedPkg.dsc
> Nt32Pkg/Nt32Pkg.dsc
> Omap35xxPkg/Omap35xxPkg.dsc
> QuarkPlatformPkg/Quark.dsc
> QuarkPlatformPkg/QuarkMin.dsc
> Vlv2TbltDevicePkg/PlatformPkgConfig.dsc
> Vlv2TbltDevicePkg/PlatformPkgGccX64.dsc
> Vlv2TbltDevicePkg/PlatformPkgIA32.dsc
> Vlv2TbltDevicePkg/PlatformPkgX64.dsc
>
> Fixing any possible warnings in these packages is left as an exercise to
> their respective maintainers.
>
> Public branch:
> <https://github.com/lersek/edk2/commits/warn_unused_but_set>.
>
> Cc: Andrew Fish <af...@apple.com>
> Cc: Ard Biesheuvel <ard.biesheu...@linaro.org>
> Cc: Feng Tian <feng.t...@intel.com>
> Cc: Hao Wu <hao.a...@intel.com>
> Cc: Jeff Fan <jeff....@intel.com>
> Cc: Jiaxin Wu <jiaxin...@intel.com>
> Cc: Jiewen Yao <jiewen....@intel.com>
> Cc: Jordan Justen <jordan.l.jus...@intel.com>
> Cc: Kelly Steele <kelly.ste...@intel.com>
> Cc: Liming Gao <liming....@intel.com>
> Cc: Maurice Ma <maurice...@intel.com>
> Cc: Michael D Kinney <michael.d.kin...@intel.com>
> Cc: Michael Kinney <michael.d.kin...@intel.com>
> Cc: Prince Agyeman <prince.agye...@intel.com>
> Cc: Ruiyu Ni <ruiyu...@intel.com>
> Cc: Siyuan Fu <siyuan...@intel.com>
> Cc: Star Zeng <star.z...@intel.com>
> Cc: Yonghong Zhu <yonghong....@intel.com>
>
> Thanks
> Laszlo
>
> Ard Biesheuvel (3):
> MdeModulePkg/PciHostBridgeDxe: remove unused but set variables
> UefiCpuPkg/MtrrLib: remove unused but set variable
> BaseTools/GCC: set -Wno-unused-but-set-variables only on RELEASE
> builds
>
> Laszlo Ersek (32):
> CorebootPayloadPkg: FbGop: remove set but unused variables
> CorebootPayloadPkg: PlatformBdsLib: remove set but unused variables
> EdkCompatibilityPkg: UefiEfiIfrSupportLib: remove set but not used
> variables
> EdkCompatibilityPkg: BsSerialStatusCode: remove set but unused
> variable
> EdkCompatibilityPkg: EdkIIGlueLib: remove set but unused variables
> EdkCompatibilityPkg: SmmBaseHelper: remove set but unused variables
> EmulatorPkg: CpuRuntimeDxe: remove set but unused variables
> IntelFrameworkModulePkg: BiosVideo: remove set but unused variable
> IntelFrameworkModulePkg: DxeCapsuleLib: remove set but unused
> variables
> IntelFrameworkModulePkg: LegacyBootMaintUiLib: remove set but unused
> variables
> IntelFspWrapperPkg: PeiFspHobProcessLibSample: remove set but unused
> variables
> MdeModulePkg: BootManagerMenuApp: remove set but unused variables
> MdeModulePkg: UfsPassThruDxe: remove set but unused variables
> MdeModulePkg: BootMaintenanceManagerUiLib: remove set but unused
> variables
> MdeModulePkg: DeviceManagerUiLib: remove set but unused variables
> NetworkPkg: IpSecDxe: remove set but unused variables
> OptionRomPkg: FtdiUsbSerialDxe: remove set but unused variables
> QuarkSocPkg: IntelQNCLib: remove set but unused variables
> QuarkSocPkg: MtrrLib: remove set but unused variables
> QuarkSocPkg: QNCSmmLib: remove set but unused variables
> QuarkSocPkg: MemoryInit/Pei: remove set but unused variable TRFC
> QuarkSocPkg: QNCInit/Dxe: remove set but unused variables
> QuarkSocPkg: SmmControlDxe: remove set but unused variables
> QuarkSocPkg: SmmControlPei: remove set but unused variables
> QuarkSocPkg: Spi/Common: remove set but unused variables
> QuarkSocPkg: SDMediaDeviceDxe: remove set but unused variables
> QuarkSocPkg: Ohci/Dxe: remove set but unused variables
> QuarkSocPkg: Ohci/Pei: remove set but unused variables
> SourceLevelDebugPkg: DebugAgentCommon: remove set but unused variables
> SourceLevelDebugPkg: DebugCommunicationLibUsb: remove set but unused
> variables
> UefiCpuPkg: PiSmmCpuDxeSmm: remove set but unused variables
> UefiCpuPkg: CpuMpPei: remove set but unused variables
>
> QuarkSocPkg/QuarkNorthCluster/Library/QNCSmmLib/QNCSmmLib.inf
> | 1 -
> CorebootPayloadPkg/FbGop/FbGop.c
> | 17 ++++-----
> CorebootPayloadPkg/Library/PlatformBdsLib/BdsPlatform.c
> | 2 --
> EdkCompatibilityPkg/Compatibility/SmmBaseHelper/SmmBaseHelper.c
> | 19 +++++------
>
> EdkCompatibilityPkg/Foundation/Library/Dxe/UefiEfiIfrSupportLib/UefiIfrCommon.c
> | 4 ---
>
> EdkCompatibilityPkg/Foundation/Library/Dxe/UefiEfiIfrSupportLib/UefiIfrForm.c
> | 4 ---
>
> EdkCompatibilityPkg/Foundation/Library/Dxe/UefiEfiIfrSupportLib/UefiIfrOpCodeCreation.c
> | 2 --
>
> EdkCompatibilityPkg/Foundation/Library/EdkIIGlueLib/Library/DxePerformanceLib/PerformanceLib.c
> | 5 ---
>
> EdkCompatibilityPkg/Sample/Platform/Generic/RuntimeDxe/StatusCode/Lib/BsSerialStatusCode/BsSerialStatusCode.c
> | 4 ---
> EmulatorPkg/CpuRuntimeDxe/Cpu.c
> | 9 -----
> IntelFrameworkModulePkg/Csm/BiosThunk/VideoDxe/BiosVideo.c
> | 13 ++++---
> IntelFrameworkModulePkg/Library/DxeCapsuleLib/DxeCapsuleLib.c
> | 2 --
> IntelFrameworkModulePkg/Library/LegacyBootMaintUiLib/LegacyBootMaintUi.c
> | 16 ---------
>
> IntelFspWrapperPkg/Library/PeiFspHobProcessLibSample/FspHobProcessLibSample.c
> | 3 +-
> MdeModulePkg/Application/BootManagerMenuApp/BootManagerMenu.c
> | 2 --
> MdeModulePkg/Bus/Pci/PciHostBridgeDxe/PciHostBridge.c
> | 8 -----
> MdeModulePkg/Bus/Ufs/UfsPassThruDxe/UfsPassThru.c
> | 4 ---
> MdeModulePkg/Bus/Ufs/UfsPassThruDxe/UfsPassThruHci.c
> | 7 +---
> MdeModulePkg/Library/BootMaintenanceManagerUiLib/BootMaintenance.c
> | 6 ----
> MdeModulePkg/Library/BootMaintenanceManagerUiLib/UpdatePage.c
> | 2 --
> MdeModulePkg/Library/DeviceManagerUiLib/DeviceManager.c
> | 2 --
> NetworkPkg/IpSecDxe/Ikev2/Utility.c
> | 2 --
> OptionRomPkg/Bus/Usb/FtdiUsbSerialDxe/FtdiUsbSerialDriver.c
> | 2 --
> QuarkSocPkg/QuarkNorthCluster/Library/IntelQNCLib/PciExpress.c
> | 13 +------
> QuarkSocPkg/QuarkNorthCluster/Library/MtrrLib/MtrrLib.c
> | 2 --
> QuarkSocPkg/QuarkNorthCluster/Library/QNCSmmLib/QNCSmmLib.c
> | 4 +--
> QuarkSocPkg/QuarkNorthCluster/MemoryInit/Pei/meminit.c
> | 3 +-
> QuarkSocPkg/QuarkNorthCluster/QNCInit/Dxe/QNCInit.c
> | 3 --
> QuarkSocPkg/QuarkNorthCluster/Smm/Dxe/SmmControlDxe/SmmControlDriver.c
> | 4 +--
> QuarkSocPkg/QuarkNorthCluster/Smm/Pei/SmmControlPei/SmmControlPei.c
> | 4 +--
> QuarkSocPkg/QuarkNorthCluster/Spi/Common/SpiCommon.c
> | 23 -------------
> QuarkSocPkg/QuarkSouthCluster/Sdio/Dxe/SDMediaDeviceDxe/CEATA.c
> | 3 --
> QuarkSocPkg/QuarkSouthCluster/Sdio/Dxe/SDMediaDeviceDxe/CEATABlockIo.c
> | 3 +-
> QuarkSocPkg/QuarkSouthCluster/Sdio/Dxe/SDMediaDeviceDxe/MMCSDTransfer.c
> | 2 --
> QuarkSocPkg/QuarkSouthCluster/Usb/Ohci/Dxe/Ohci.c
> | 9 -----
> QuarkSocPkg/QuarkSouthCluster/Usb/Ohci/Dxe/OhciReg.c
> | 5 +--
> QuarkSocPkg/QuarkSouthCluster/Usb/Ohci/Pei/OhcPeim.c
> | 10 ------
> QuarkSocPkg/QuarkSouthCluster/Usb/Ohci/Pei/OhciReg.c
> | 2 --
> SourceLevelDebugPkg/Library/DebugAgent/DebugAgentCommon/DebugAgent.c
> | 3 --
>
> SourceLevelDebugPkg/Library/DebugCommunicationLibUsb/DebugCommunicationLibUsb.c
> | 14 +-------
> UefiCpuPkg/CpuMpPei/CpuMpPei.c
> | 3 +-
> UefiCpuPkg/Library/MtrrLib/MtrrLib.c
> | 2 --
> UefiCpuPkg/PiSmmCpuDxeSmm/SmmProfile.c
> | 34 ++++++------------
> BaseTools/Conf/tools_def.template
> | 36 ++++++++++++++------
> 44 files changed, 68 insertions(+), 250 deletions(-)
>
> --
> 1.8.3.1
>
_______________________________________________
edk2-devel mailing list
edk2-devel@lists.01.org
https://lists.01.org/mailman/listinfo/edk2-devel
