[edk2-devel] [PATCH v2 2/3] UefiPayloadPkg: Add PayloadLoaderPeim which can load ELF payload

[Marvin Häuser]

Hey Ray,

Sure, thanks a lot for taking the time. I will need a bit longer to get 
to it, sorry. :)

Best regards,
Marvin

On 15.06.21 16:36, Ni, Ray wrote:
> Marvin,
> I have sent out https://edk2.groups.io/g/devel/message/76429 <UefiPayloadPkg/PayloadLoader: Add more checks to verify ELF images> to address your feedbacks.
>
> Can I merge the 3 patches first? (we can continue discussing the more-checks patch.)
>
> Thanks,
> Ray
>
>> -----Original Message-----
>> From: devel at edk2.groups.io <devel at edk2.groups.io> On Behalf Of Ni, Ray
>> Sent: Thursday, June 10, 2021 7:37 PM
>> To: devel at edk2.groups.io; mcb30 at ipxe.org; mhaeuser at posteo.de
>> Cc: Ma, Maurice <maurice.ma at intel.com>; Dong, Guo <guo.dong at intel.com>; You, Benjamin <benjamin.you at intel.com>
>> Subject: Re: [edk2-devel] [PATCH v2 2/3] UefiPayloadPkg: Add PayloadLoaderPeim which can load ELF payload
>>
>>
>>
>>> -----Original Message-----
>>> From: devel at edk2.groups.io <devel at edk2.groups.io> On Behalf Of Michael Brown
>>> Sent: Thursday, June 10, 2021 6:43 PM
>>> To: devel at edk2.groups.io; mhaeuser at posteo.de; Ni, Ray <ray.ni at intel.com>
>>> Cc: Ma, Maurice <maurice.ma at intel.com>; Dong, Guo <guo.dong at intel.com>; You, Benjamin <benjamin.you at intel.com>
>>> Subject: Re: [edk2-devel] [PATCH v2 2/3] UefiPayloadPkg: Add PayloadLoaderPeim which can load ELF payload
>>>
>>> On 10/06/2021 11:13, Marvin Häuser wrote:
>>>> On 10.06.21 11:39, Ni, Ray wrote:
>>>>>> Maybe for some context, my main issue at first was that the checks are
>>>>>> all proper runtime checks with no ASSERTs at all, so I got confused how
>>>>>> this situation could happen in a realistic scenario. I needed to trace
>>>>>> the ParseStatus data flow to understand the idea is basically the same
>>>>>> as in the PE library. Code in a way is self-documenting, and this
>>>>>> personally gave me a hard time understanding why it is written this way.
>>>>>> But thanks for clarifying your intention! :)
>>>>> I assume you are ok with the ParseStatus.
>>>>> I will send new version based on mail discussion. Thanks!
>>>> I don't need to be okay with anything, I'm not a maintainer nor an
>>>> authority. But I gave my opinion, which is that it is dead code that
>>>> makes the design/flow harder to understand for a third party, at no
>>>> obvious benefit.
>>> FWIW, I strongly agree with Marvin on this: having ParseStatus in its
>>> current form is a bad idea since it adds no value but does incur a cost.
>> OK. I can remove that😊
>>
>>
>>
>> 
>>



-=-=-=-=-=-=-=-=-=-=-=-
Groups.io Links: You receive all messages sent to this group.
View/Reply Online (#76555): https://edk2.groups.io/g/devel/message/76555
Mute This Topic: https://groups.io/mt/83277976/1813853
Group Owner: devel+owner at edk2.groups.io
Unsubscribe: https://edk2.groups.io/g/devel/unsub [edk2-devel-archive at redhat.com]
-=-=-=-=-=-=-=-=-=-=-=-