The correct log should be:
  - The SectionAlignment is got from Magic number.
  - The Magic number is got from PE header and machine type.

The original code mix them.

Thank you
Yao Jiewen

-----Original Message-----
From: Yao, Jiewen 
Sent: Tuesday, June 09, 2015 8:42 PM
To: 'Ard Biesheuvel'
Cc: [email protected]; Zeng, Star; Gao, Liming
Subject: RE: [edk2] [patch] MdePkg/PropertiesTable support, 
MdeModulePkg/DxeCore support UEFI2.5 properties table

Hi Ard
You are right. I happen to find a logic error, too.

Attach patch for your review.



-----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

Reply via email to