On Thu, 13 Nov 2014 09:27:09 -0600, Scott Duplichan wrote: > Gabriel Somlo [mailto:[email protected]] wrote: > ]On Sun, Nov 09, 2014 at 05:00:38PM -0800, Jordan Justen wrote: > ]> On 2014-11-08 14:01:56, Gabriel L. Somlo wrote: > ]> > @@ -228,6 +229,11 @@ MiscInitialization ( > ]> > VOID > ]> > ) > ]> > { > ]> > + UINT16 HostBridgeDevId; > ]> > + UINTN PmCmd; > ]> > + UINTN Pmba; > ]> > + UINTN PmRegMisc; > ]> > + > ]> > // > ]> > // Disable A20 Mask > ]> > // > ]> > @@ -239,33 +245,47 @@ MiscInitialization ( > ]> > BuildCpuHob (36, 16); > ]> > > ]> > // > ]> > + // Query Host Bridge DID to determine platform type > ]> > + // > ]> > + HostBridgeDevId = PciRead16 (OVMF_HOSTBRIDGE_DID); > ]> > + switch (HostBridgeDevId) { > ]> > + case INTEL_82441_DEVICE_ID: > ]> > + PmCmd = POWER_MGMT_REGISTER_PIIX4 (PCI_COMMAND_OFFSET); > ]> > + Pmba = POWER_MGMT_REGISTER_PIIX4 (0x40); > ]> > + PmRegMisc = POWER_MGMT_REGISTER_PIIX4 (0x80); > ]> > + break; > ]> > + case INTEL_Q35_MCH_DEVICE_ID: > ]> > + PmCmd = POWER_MGMT_REGISTER_Q35 (PCI_COMMAND_OFFSET); > ]> > + Pmba = POWER_MGMT_REGISTER_Q35 (0x40); > ]> > + PmRegMisc = POWER_MGMT_REGISTER_Q35 (0x80); > ]> > + break; > ]> > + default: > ]> > + DEBUG ((EFI_D_ERROR, "%a: Unknown Host Bridge Device ID: > 0x%04x\n", > ]> > + __FUNCTION__, HostBridgeDevId)); > ]> > + ASSERT (FALSE); > ]> > ]> When I build IA32 (OvmfPkg/build.sh -a IA32) I get warnings about > ]> possibly uninitialized variables. > ]> > ]> It seems to happen basically everwhere that you switch on > ]> HostBridgeDevId. > ] > ]I wonder why it's only warning on IA32, not like its different on X64 :) > > Because GCC49 IA32 is an optimized build (-Os) and GCC49 X64 is not.
OK, thanks, that makes sense :) > ]Assuming we're still on the same page, the question becomes how to > ]shut up gcc. > > Using a phrase like 'shut up gcc' sort of implies the warning is in > the bogus category, like so many from the Microsoft compilers. But > this warning is valid, right? For example, if I run Ovmf in some > environment where PIIX4 is not present, then the code will fail > to find the ACPI timer I/O address. Why not return an error if > no supported device ID is found. That will eliminate the warning, > and prevent the code from running on the unsupported chipset. I'm using this type of switch statement from several places, some of which return VOID. Right now, that's OvmfPkg/PlatformPei/Platform.c MiscInitialization () OvmfPkg/Csm/CsmSupportLib/LegacyInterrupt.c LegacyInterruptInstall () OvmfPkg/Library/AcpiTimerLib/*AcpiTimerLib.c AcpiTimerLibConstructor () OvmfPkg/Library/PlatformBdsLib/BdsPlatform.c PciAcpiInitialization () I could add a "return EFI_UNSUPPORTED" for those of them which return a status code, or simply a "return" for those of type VOID, however: I'd still basically be just "shutting up gcc", because I'd place these return statements after the ASSERT (FALSE). Once neither host bridge type we expect has been detected, the world ends and returning "for real" makes no sense, as we're in an unexpected/undefined state. Hence the ASSERT. Jordan, what do you think ? Adding a "return [EFI_UNSUPPORTED]" after the ASSERT (FALSE), is that stylistically preferable over zeroing out the values before the switch() ? Thanks, --Gabriel ------------------------------------------------------------------------------ Comprehensive Server Monitoring with Site24x7. Monitor 10 servers for $9/Month. Get alerted through email, SMS, voice calls or mobile push notifications. Take corrective actions from your mobile device. http://pubads.g.doubleclick.net/gampad/clk?id=154624111&iu=/4140/ostg.clktrk _______________________________________________ edk2-devel mailing list [email protected] https://lists.sourceforge.net/lists/listinfo/edk2-devel
