On 9 June 2015 at 14:42, Yao, Jiewen <jiewen....@intel.com> wrote: > Hi Ard > You are right. I happen to find a logic error, too. > > Attach patch for your review. >
Yes, that looks fine to me, although it is unfortunate that can't just use PeCoffLoaderGetPeHeaderMagicValue () directly. It builds fine on GCC/AArch64 Reviewed-by: Ard Biesheuvel <ard.biesheu...@linaro.org> Thanks, Ard. > > > -----Original Message----- > From: Ard Biesheuvel [mailto:ard.biesheu...@linaro.org] > Sent: Tuesday, June 09, 2015 8:28 PM > To: Yao, Jiewen > Cc: edk2-devel@lists.sourceforge.net; Zeng, Star; Gao, Liming > Subject: Re: [edk2] [patch] MdePkg/PropertiesTable support, > MdeModulePkg/DxeCore support UEFI2.5 properties table > > On 5 June 2015 at 15:22, Ard Biesheuvel <ard.biesheu...@linaro.org> wrote: >> On 5 June 2015 at 15:21, Yao, Jiewen <jiewen....@intel.com> wrote: >>> Hi Ard >>> Thanks to report this. I have fixed this in 17565. >> >> Wow, that was quick! Thanks! >> > > You have fixed the build, but the copy-pasted comments are still there, which > make no sense at all anymore, since they refer to the Magic variable, which > has been removed. > > Next time, could you please send your patches (including fixes like > these) to the mailing list first? That way, people have a chance to see > what's going in before its gets merged. > > Thanks, > Ard. > > >> >>> -----Original Message----- >>> From: Ard Biesheuvel [mailto:ard.biesheu...@linaro.org] >>> Sent: Friday, June 05, 2015 9:12 PM >>> To: edk2-devel@lists.sourceforge.net; Yao, Jiewen; Zeng, Star; Gao, >>> Liming >>> Subject: Re: [edk2] [patch] MdePkg/PropertiesTable support, >>> MdeModulePkg/DxeCore support UEFI2.5 properties table >>> >>> On 4 June 2015 at 16:34, Yao, Jiewen <jiewen....@intel.com> wrote: >>>> Hi >>>> >>>> Here is patch to support UEFI2.5, chapter 4, section 4.6 Properties >>>> Table feature. >>>> >>>> >>>> >>>> MdePkg – add Properties table definition. >>>> >>>> MdeModulePkg – add properties table support in DXE core. >>>> >>>> MdeModulePkg – add PropertiesTableAttributesDxe driver to set >>>> ACPINvs/Reserved memory type to be XP. >>>> >>>> >>>> >>>> Signed-off-by: Yao, Jiewen jiewen....@intel.com >>>> >>>> Reviewed-by: Zeng, Star star.z...@intel.com >>>> >>>> Reviewed-by: Gao, Liming liming....@intel.com >>>> >>> >>> This patch breaks the build for GCC on 64-bit ARM >>> >>> 13:03:43 >>> /home/buildslave/workspace/leg-virt-tianocore-edk2-upstream/edk2/MdeModulePkg/Core/Dxe/Misc/PropertiesTable.c: >>> In function ‘InsertImageRecord’: >>> 13:03:43 >>> /home/buildslave/workspace/leg-virt-tianocore-edk2-upstream/edk2/MdeModulePkg/Core/Dxe/Misc/PropertiesTable.c:1113:10: >>> error: variable ‘Magic’ set but not used [-Werror=unused-but-set-variable] >>> 13:03:43 UINT16 Magic; >>> 13:03:43 ^ >>> 13:03:43 >>> /home/buildslave/workspace/leg-virt-tianocore-edk2-upstream/edk2/MdeModulePkg/Core/Dxe/Misc/PropertiesTable.c:1110:10: >>> error: variable ‘ImageType’ set but not used >>> [-Werror=unused-but-set-variable] >>> 13:03:43 UINT16 ImageType; >>> 13:03:43 ^ >>> >>> >>> and honestly, i can't be bothered to look at the code myself, considering >>> the way it was dumped into the mailing list and the repository. >>> >>> Please get this fixed >>> >>> Regards, >>> Ard. >>> >>> >>>> >>>> -------------------------------------------------------------------- >>>> -- >>>> -------- >>>> >>>> _______________________________________________ >>>> edk2-devel mailing list >>>> edk2-devel@lists.sourceforge.net >>>> https://lists.sourceforge.net/lists/listinfo/edk2-devel >>>> ------------------------------------------------------------------------------ _______________________________________________ edk2-devel mailing list edk2-devel@lists.sourceforge.net https://lists.sourceforge.net/lists/listinfo/edk2-devel
