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@edk2.groups.io <devel@edk2.groups.io> On Behalf Of Ni, Ray > Sent: Thursday, June 10, 2021 7:37 PM > To: devel@edk2.groups.io; mc...@ipxe.org; mhaeu...@posteo.de > Cc: Ma, Maurice <maurice...@intel.com>; Dong, Guo <guo.d...@intel.com>; You, > Benjamin <benjamin....@intel.com> > Subject: Re: [edk2-devel] [PATCH v2 2/3] UefiPayloadPkg: Add > PayloadLoaderPeim which can load ELF payload > > > > > -----Original Message----- > > From: devel@edk2.groups.io <devel@edk2.groups.io> On Behalf Of Michael Brown > > Sent: Thursday, June 10, 2021 6:43 PM > > To: devel@edk2.groups.io; mhaeu...@posteo.de; Ni, Ray <ray...@intel.com> > > Cc: Ma, Maurice <maurice...@intel.com>; Dong, Guo <guo.d...@intel.com>; > > You, Benjamin <benjamin....@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/21656 Group Owner: devel+ow...@edk2.groups.io Unsubscribe: https://edk2.groups.io/g/devel/unsub [arch...@mail-archive.com] -=-=-=-=-=-=-=-=-=-=-=-
