Lzalo: This patch includes the change to *_ELFGCC_X64_CC_FLAGS. But, no changes is for XCLANG and XCODE. If Andrew agrees this change, I can apply it in XCLANG and XCODE tool chain.
4) I only verify it in common package. Thanks for your test on other platforms. Your patch for OVMF platform is good to me. Reviewed-by: Gao, Liming <liming....@intel.com> Thanks Liming -----Original Message----- From: Olivier Martin [mailto:olivier.mar...@arm.com] Sent: Friday, June 20, 2014 9:30 PM To: edk2-devel@lists.sourceforge.net; Gao, Liming; edk2-buildtools-de...@lists.sourceforge.net Subject: RE: [edk2] [edk2-buildtools] [Patch] Remove GCC option -Wno-missing-braces option Thanks Lazlo, I was actually doing the same exercise with our build system. I am still going through them... But as far as I have seen, there is no error int the common packages. So the patch looks fine to me. Reviewed-By: Olivier Martin <olivier.mar...@arm.com> Tested-By: Olivier Martin <olivier.mar...@arm.com> > -----Original Message----- > From: Laszlo Ersek [mailto:ler...@redhat.com] > Sent: 20 June 2014 14:17 > To: Gao, Liming; edk2-buildtools-de...@lists.sourceforge.net; edk2- > de...@lists.sourceforge.net > Subject: Re: [edk2] [edk2-buildtools] [Patch] Remove GCC option -Wno- > missing-braces option > > On 06/20/14 12:00, Gao, Liming wrote: > > Hi, all > > > > -Wno-missing-braces option is used to disable warning when the > > initialized value without braces is set into structure. To remove > this > > option can detect such wrong case in source code. This patch remove > it > > from the default GCC compiler option in > > BaseTools/Conf/tools_def.template. Please help review it. > > (1) The patch makes sense to me, because a human should arguably > prefer > > int b[2][2] = { { 0, 1 }, { 2, 3 } }; > > over > > int a[2][2] = { 0, 1, 2, 3 }; > > It could be a little inconvenient for machine generated code though. > (Especially that the warning will be treated as an error.) But I guess > it should be fine. > > (2) The patch removes the flag from two GCC defines (GCC_ALL_CC_FLAGS > and GCC44_ALL_CC_FLAGS). In my edk2 clone there's a third, similar > flag: > "*_ELFGCC_X64_CC_FLAGS". Did you leave that out deliberately? > > (3) Other compilers seem to retain -Wno-missing-braces (eg. XCLANG, > XCODE). Is that intentional, or are they not modified only because > it's hard to test them? (I don't mind if the latter is the case, I'd > just like to understand.) > > (4) In order to test the patch, I grabbed my existing > "Conf/tools_def.txt" and removed all instances of -Wno-missing-braces. > > Then I tried to build OVMF (for both Ia32 and X64), and also > (cross-build) "ArmVExpress-FVP-AArch64" and > "ArmVExpress-RTSM-AEMv8Ax4-foundation". > > The change triggers a few build errors in OVMF, and many more in the > ARM packages. Please see my attached fixes. I propose that these > patches be applied first (after review), before committing & syncing > this BaseTools change. > > Thanks > Laszlo ------------------------------------------------------------------------------ HPCC Systems Open Source Big Data Platform from LexisNexis Risk Solutions Find What Matters Most in Your Big Data with HPCC Systems Open Source. Fast. Scalable. Simple. Ideal for Dirty Data. Leverages Graph Analysis for Fast Processing & Easy Data Exploration http://p.sf.net/sfu/hpccsystems _______________________________________________ edk2-devel mailing list edk2-devel@lists.sourceforge.net https://lists.sourceforge.net/lists/listinfo/edk2-devel
