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

[Ni, Ray]

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 (#76521): https://edk2.groups.io/g/devel/message/76521
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]
-=-=-=-=-=-=-=-=-=-=-=-