(adding Ard and Shumin because the below will tie in with another thread) On 03/17/16 19:05, Leif Lindholm wrote:
> I must confess to no small amount of surprise that optionally adding > the ability to tag an unused argument as unused is controversial. I'm also surprised, but by a different thing. :) (1) This is a side point, but I think it is a bit relevant. See this sub-thread: http://thread.gmane.org/gmane.comp.bios.edk2.devel/8848/focus=9254 Shumin did build his patch in question with GCC46, but the warning was not reported to him. (Side side point: I will assume that Shumin is a male name. I googled it and found nothing conclusive; apologies!) Ard and myself stumbled upon it because we built the code for aarch64 as well. So the difference is ("obviously" in retrospect) in the different build flags. Compare, in "BaseTools/Conf/tools_def.template": DEFINE GCC46_IA32_CC_FLAGS = DEF(GCC45_IA32_CC_FLAGS) -Wno-address -Wno-unused-but-set-variable DEFINE GCC46_X64_CC_FLAGS = DEF(GCC45_X64_CC_FLAGS) -Wno-address -Wno-unused-but-set-variable vs. DEBUG_GCC48_AARCH64_CC_FLAGS = DEF(GCC48_AARCH64_CC_FLAGS) -O0 RELEASE_GCC48_AARCH64_CC_FLAGS = DEF(GCC48_AARCH64_CC_FLAGS) -Wno-unused-but-set-variable Only the DEBUG target for AARCH64 does not include "-Wno-unused-but-set-variable"; that's why Ard and I got that warning, and Shumin didn't. I think it might be feasible to remove -Wno-unused-but-set-variable. I removed it and built OvmfPkg and ArmVirtPkg with a number of settings (for the DEBUG target, with gcc-4.8). The warnings that surfaced were only: > MdeModulePkg/Bus/Pci/PciHostBridgeDxe/PciHostBridge.c:1114:29: warning: > variable 'BusEnd' set but not used [-Wunused-but-set-variable] > MdeModulePkg/Bus/Pci/PciHostBridgeDxe/PciHostBridge.c:1114:9: warning: > variable 'BusEnd' set but not used [-Wunused-but-set-variable] > MdeModulePkg/Bus/Pci/PciHostBridgeDxe/PciHostBridge.c:631:10: warning: > variable 'AddrLen' set but not used [-Wunused-but-set-variable] > MdeModulePkg/Bus/Pci/PciHostBridgeDxe/PciHostBridge.c:631:41: warning: > variable 'AddrLen' set but not used [-Wunused-but-set-variable] > UefiCpuPkg/Library/MtrrLib/MtrrLib.c:462:11: warning: variable 'TempQword' > set but not used [-Wunused-but-set-variable] > UefiCpuPkg/PiSmmCpuDxeSmm/SmmProfile.c:1314:25: warning: variable > 'SwSmiCpuIndex' set but not used [-Wunused-but-set-variable] > UefiCpuPkg/PiSmmCpuDxeSmm/SmmProfile.c:1314:9: warning: variable > 'SwSmiCpuIndex' set but not used [-Wunused-but-set-variable] > UefiCpuPkg/PiSmmCpuDxeSmm/SmmProfile.c:345:14: warning: variable 'Status' set > but not used [-Wunused-but-set-variable] > UefiCpuPkg/PiSmmCpuDxeSmm/SmmProfile.c:345:36: warning: variable 'Status' set > but not used [-Wunused-but-set-variable] > UefiCpuPkg/PiSmmCpuDxeSmm/SmmProfile.c:779:14: warning: variable 'Status' set > but not used [-Wunused-but-set-variable] > UefiCpuPkg/PiSmmCpuDxeSmm/SmmProfile.c:779:22: warning: variable 'Status' set > but not used [-Wunused-but-set-variable] Side point ends. (2) Beyond removing "-Wno-unused-but-set-variable", I then added "-Wunused-parameter -Wunused-but-set-parameter". Oh boy. :) First, a large number of "AutoGen.c" files (maybe all of them?) seem to hit it. I simply filtered those out. Second, the remaining set of warnings is also huge: almost 4000 instances. The list of locations is too large to attach or paste (and I don't think the list will allow compressed attachments), so I'm uploading it here: <http://people.redhat.com/lersek/unused-parameter.txt>. If I understand correctly, if we wanted to enable "-Wunused-parameter -Wunused-but-set-parameter" even just occasionally, these ~4000 instances would have to be audited, and each should be either fixed (i.e., internal functions should drop the parameters) or marked UNUSED (i.e., library instances and PPI/protocol implementations should annotate their definitions of public functions). Thus, this is what surprises me. It looks daunting. Thanks Laszlo _______________________________________________ edk2-devel mailing list [email protected] https://lists.01.org/mailman/listinfo/edk2-devel
