Thank you! Agree, I have similar concern on PeCoffLoaderGetPeHeaderMagicValue().
I have seen duplicated code in below modules:
1) SecurityPkg\Library\DxeImageVerificationLib\DxeImageVerificationLib.c(363):
if (mNtHeader.Pe32->FileHeader.Machine == IMAGE_FILE_MACHINE_IA64 &&
mNtHeader.Pe32->OptionalHeader.Magic == EFI_IMAGE_NT_OPTIONAL_HDR32_MAGIC) {
2) SecurityPkg\Library\DxeImageVerificationLib\DxeImageVerificationLib.c(1690):
if (mNtHeader.Pe32->FileHeader.Machine == IMAGE_FILE_MACHINE_IA64 &&
mNtHeader.Pe32->OptionalHeader.Magic == EFI_IMAGE_NT_OPTIONAL_HDR32_MAGIC) {
3) SecurityPkg\Library\DxeTpmMeasureBootLib\DxeTpmMeasureBootLib.c(419): if
(Hdr.Pe32->FileHeader.Machine == IMAGE_FILE_MACHINE_IA64 &&
Hdr.Pe32->OptionalHeader.Magic == EFI_IMAGE_NT_OPTIONAL_HDR32_MAGIC) {
4) SecurityPkg\Tcg\TrEEDxe\MeasureBootPeCoff.c(114): if
(Hdr.Pe32->FileHeader.Machine == IMAGE_FILE_MACHINE_IA64 &&
Hdr.Pe32->OptionalHeader.Magic == EFI_IMAGE_NT_OPTIONAL_HDR32_MAGIC) {
5)
SecurityPkg\VariableAuthenticated\SecureBootConfigDxe\SecureBootConfigImpl.c(1734):
if (mNtHeader.Pe32->FileHeader.Machine == IMAGE_FILE_MACHINE_IA64 &&
mNtHeader.Pe32->OptionalHeader.Magic == EFI_IMAGE_NT_OPTIONAL_HDR32_MAGIC) {
How I wish it is exposed!
Thank you
Yao Jiewen
-----Original Message-----
From: Ard Biesheuvel [mailto:[email protected]]
Sent: Tuesday, June 09, 2015 9:22 PM
To: Yao, Jiewen
Cc: [email protected]; Zeng, Star; Gao, Liming
Subject: Re: [edk2] [patch] MdePkg/PropertiesTable support,
MdeModulePkg/DxeCore support UEFI2.5 properties table
On 9 June 2015 at 14:42, Yao, Jiewen <[email protected]> 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 <[email protected]>
Thanks,
Ard.
>
>
> -----Original Message-----
> From: Ard Biesheuvel [mailto:[email protected]]
> Sent: Tuesday, June 09, 2015 8:28 PM
> To: Yao, Jiewen
> Cc: [email protected]; 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 <[email protected]> wrote:
>> On 5 June 2015 at 15:21, Yao, Jiewen <[email protected]> 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:[email protected]]
>>> Sent: Friday, June 05, 2015 9:12 PM
>>> To: [email protected]; 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 <[email protected]> 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 [email protected]
>>>>
>>>> Reviewed-by: Zeng, Star [email protected]
>>>>
>>>> Reviewed-by: Gao, Liming [email protected]
>>>>
>>>
>>> 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
>>>> [email protected]
>>>> https://lists.sourceforge.net/lists/listinfo/edk2-devel
>>>>
------------------------------------------------------------------------------
_______________________________________________
edk2-devel mailing list
[email protected]
https://lists.sourceforge.net/lists/listinfo/edk2-devel
