From mboxrd@z Thu Jan 1 00:00:00 1970 Received: from mail.byosoft.com.cn (mail.byosoft.com.cn [58.240.74.242]) by mx.groups.io with SMTP id smtpd.web12.2804.1632876681845037395 for ; Tue, 28 Sep 2021 17:51:24 -0700 Authentication-Results: mx.groups.io; dkim=missing; spf=none, err=permanent DNS error (domain: byosoft.com.cn, ip: 58.240.74.242, mailfrom: gaoliming@byosoft.com.cn) Received: from DESKTOPS6D0PVI ([58.246.60.130]) (envelope-sender ) by 192.168.6.13 with ESMTP for ; Wed, 29 Sep 2021 08:51:03 +0800 X-WM-Sender: gaoliming@byosoft.com.cn X-Originating-IP: 58.246.60.130 X-WM-AuthFlag: YES X-WM-AuthUser: gaoliming@byosoft.com.cn From: "gaoliming" To: "'Brian J. Johnson'" , , , =?UTF-8?Q?'Marvin_H=C3=A4user'?= , , "'Chan, Amy'" , "'Andrew Fish'" Cc: "'Kinney, Michael D'" , "'Gao, Liming'" , "'Liu, Zhiguang'" , "'Wang, Jian J'" , "'Gao, Zhichao'" References: <001a01d7aa99$d744af80$85ce0e80$@byosoft.com.cn> <003401d7aaa1$c7166830$55433890$@byosoft.com.cn> <16A70FAC1585C7C1.27516@groups.io> <00cf01d7b27a$59145d20$0b3d1760$@byosoft.com.cn> <2021092708451371303013@byosoft.com.cn> <7c260e5d-363e-8579-5a38-3cc5e2336bdb@posteo.de> <2021092716501903570820@byosoft.com.cn> <57dc418a-6620-629c-c448-777994c280d6@posteo.de> <7d30e188-38b1-76cb-2e55-6f3a82338473@hpe.co m> In-Reply-To: <7d30e188-38b1-76cb-2e55-6f3a82338473@hpe.com> Subject: =?UTF-8?B?5Zue5aSNOiBbZWRrMi1kZXZlbF0gUkZDOiBBZGQgQmFzZUxpYi9RdWlja1NvcnQgaW4gTWRlUGtn?= Date: Wed, 29 Sep 2021 08:51:04 +0800 Message-ID: <012d01d7b4cc$152bf790$3f83e6b0$@byosoft.com.cn> MIME-Version: 1.0 X-Mailer: Microsoft Outlook 16.0 Thread-Index: AQLnLItH7GrFLJqnU7rRMhL6o/j0rALHrNCxAkD3tuQBn1kvPgNjcDkYAU1KPNYBRs4xIAJq1+G/AnJN82sCRPmMgwLB+OO0AiU7PncBBuNMSAGmGWA2AX1RFYcBxSaREwIctDJ5qJWW1sA= Content-Type: text/plain; charset="UTF-8" Content-Transfer-Encoding: quoted-printable Content-Language: zh-cn Johnson: I also agree this proposal to make BufferOneElement parameter mandatory. Thanks Liming > -----=E9=82=AE=E4=BB=B6=E5=8E=9F=E4=BB=B6----- > =E5=8F=91=E4=BB=B6=E4=BA=BA: Brian J. Johnson > =E5=8F=91=E9=80=81=E6=97=B6=E9=97=B4: 2021=E5=B9=B49=E6=9C=8829=E6=97=A5 = 6:26 > =E6=94=B6=E4=BB=B6=E4=BA=BA: devel@edk2.groups.io; ray.ni@intel.com; Marv= in H=C3=A4user > ; fanjianfeng@byosoft.com.cn; 'gaoliming' > ; Chan, Amy ; 'Andrew > Fish' > =E6=8A=84=E9=80=81: Kinney, Michael D ; 'Gao,= Liming' > ; Liu, Zhiguang ; Wang, Jia= n > J ; Gao, Zhichao > =E4=B8=BB=E9=A2=98: Re: [edk2-devel] RFC: Add BaseLib/QuickSort in MdePkg >=20 > I'll add my agreement to Marvin and Jeff: a low-level sort routine like > this should let the caller be in charge of memory allocation, so it can > be used in the widest variety of contexts (SEC, exception handlers, APs, > etc.) So let's make the BufferOneElement parameter mandatory. >=20 > Brian J. Johnson >=20 > -------- Original Message -------- > From: Ni, Ray [mailto:ray.ni@intel.com] > Sent: Monday, September 27, 2021, 8:49 PM > To: Marvin H=C3=A4user , fanjianfeng@byosoft.com.cn > , devel@edk2.groups.io > , 'gaoliming' , Chan, > Amy , 'Andrew Fish' > Cc: Kinney, Michael D , 'Gao, Liming' > , Liu, Zhiguang , Wang, > Jian J , Gao, Zhichao > Subject: [edk2-devel] RFC: Add BaseLib/QuickSort in MdePkg >=20 > Marvin, > I agree with your concerns, in a certain level. But I didn't treat it > as a very big problem of having caller pass the BufferOneElement > "intelligently". >=20 > I am ok to use your option 1), making BufferOneElement mandatory. > Caller should always supply the buffer that's sufficient to hold one > element. >=20 > By the way, I don't want to propose "swap-without-using-temporary-value" > method to avoid using the "BufferOneElement"? > Because that makes the simple thing complicated! >=20 > Thanks, > Ray >=20 > > -----Original Message----- > > From: Marvin H=C3=A4user > > Sent: Monday, September 27, 2021 5:09 PM > > To: fanjianfeng@byosoft.com.cn; devel@edk2.groups.io; Ni, Ray > ; 'gaoliming' > > ; Chan, Amy ; 'Andrew > Fish' > > Cc: Kinney, Michael D ; 'Gao, Liming' > ; Liu, Zhiguang > > ; Wang, Jian J ; Gao, > Zhichao > > Subject: Re: [edk2-devel] RFC: Add BaseLib/QuickSort in MdePkg > > > > On 27/09/2021 10:50, fanjianfeng@byosoft.com.cn wrote: > >> For former caller, they could still keep as is to call the old API in > >> MdeModulePkg. I think Ray's design is compatible change for existing c= ode. > >> Only when the existing code wants to remove the dependency on > >> MdeModuelPkg, they could migrate to the new API in baselib. > >> > >> I agree with one split-API desgin what you mentioned. > >> But my point is to define one API in baselib and to make the baselib > >> not depend on memory allocation. And another wrapper API could be > >> designed to be placed in any other class. > > > > Oh sure, it could totally live in another class. I'd just like to have > > those two models (caller- and callee-owned buffer) strictly separate, I > > personally do not mind where exactly they are implemented. Thanks! > > > > Best regards, > > Marvin > > > >> > >> ----------------------------------------------------------------------= -- > >> Jeff > >> fanjianfeng@byosoft.com.cn > >> > >> *From:* Marvin H=C3=A4user > >> *Date:* 2021-09-27 16:14 > >> *To:* fanjianfeng@byosoft.com.cn > >> ; devel@edk2.groups.io > >> ; Ni, Ray ; > >> 'gaoliming' ; Chan, Amy > >> ; 'Andrew Fish' > > >> *CC:* Kinney, Michael D ; > 'Gao, > >> Liming' ; Liu, Zhiguang > >> ; Wang, Jian J > >> ; Gao, Zhichao > >> > >> *Subject:* Re: [edk2-devel] RFC: Add BaseLib/QuickSort in > MdePkg > >> On 27/09/2021 02:45, fanjianfeng@byosoft.com.cn wrote: > >> > Making baselib implementation depend on MemoryAllocationLib > >> > (indirectly on Pei Service and gBS), it may prevent > >> > this base API using at some seneraio. i don't think it's better= . > >> That is why I asked about a split-API scenario, one of which does > not > >> depend on dynamic memory allocation (SetVariable-like) and one > does > >> (wrapper-like). > >> > Add this parameter and make this parameter is optional, > >> > 1, when NULL, use the local 256 bytes stack > >> > 2, if 256 bytes stack is not enough, return > RETURE_BUFFER_TOO_SAMLL, > >> > 3, caller check the return status, to do nothing or to allocate > >> enough > >> > buffer for retry > >> > > >> > This is just like SetVariable()'s implementation. > >> Yes, and because that is SetVariable's implementation, we have > >> library > >> functions to make it less error-prone and more convenient [1]. As= a > >> matter of fact, we have a (semi-lax) policy in our codebases to a= void > >> such functions like the plague and use those library wrappers > >> where-ever > >> it can make sense. The only super-rare exceptions I can think of = are > >> when we know the size of the element ahead of time. Also > >> SetVariable has > >> no arbitrary constraint on when it may work the first time, and > >> there is > >> code that will fail when the first return is not > EFI_BUFFER_TOO_SMALL. > >> This solution honestly yields even more problems, because it > >> introduces > >> a Status return which was not there before. For common code > safety > >> and > >> quality policy, this requires the value *must* be retrieved and > >> checked > >> in some fashion. So all callers, no matter the prior knowledge of= the > >> element size, now need either a runtime check and handling for a > >> status > >> that they (may) know can never be returned, or ASSERTs if the > >> function > >> spec really imposes the arbitrary 256 Bytes constraint. Latter > >> doesn't > >> really work I think. What if someone wants to sort in SEC and > noticed > >> they only have 64 Bytes on the stack to work with, realistically?= Any > >> code depending on the 256 Byte constraint, passing NULL and not > doing > >> additional handling, would seize to work. Former is too > >> complicated, see > >> wrappers. In my opinion, the memory must *either* be fully > managed by > >> the caller *or* the callee (and you may have both in separate > >> functions, > >> as I suggested), but not sometimes here, sometimes there. > >> Best regards, > >> Marvin > >> [1] > >> > > > https://github.com/tianocore/edk2/blob/46b4606ba23498d3d0e66b53e498e > b3d5d592586/MdePkg/Library/UefiLib/UefiLib.c#L > > 1309-L1360 > >> > > >> > > >> -----------------------------------------------------------------= ------- > >> > Jeff > >> > fanjianfeng@byosoft.com.cn > >> > > >> > *From:* Marvin H=C3=A4user > >> > *Date:* 2021-09-26 19:20 > >> > *To:* devel ; ray.ni > >> > ; gaoliming > >> > ; Chan, Amy > >> > ; 'Andrew Fish' > >> > >> > *CC:* Kinney, Michael D > ; > >> 'Gao, > >> > Liming' ; Liu, Zhiguang > >> > ; Wang, Jian J > >> > ; Gao, Zhichao > >> > > >> > *Subject:* Re: [edk2-devel] RFC: Add BaseLib/QuickSort > in MdePkg > >> > Hey Ray, > >> > In my opinion that spec is too complicated. For some cases > it is > >> > obvious, but I think the last anyone wants to see is a > >> > (STATIC_)ASSERT > >> > before most QuickSort calls to ensure the element size > >> *really* is <=3D > >> > 256 Bytes. In my opinion, there are two roads: > >> > 1) Make the parameter required (I think this is what Liming > >> > suggested). > >> > The caller would always need to provide said buffer, and it > >> can do > >> > as it > >> > sees fit - on the stack, in a pool, in a page, who knows. > >> > 2) Remove the parameter entirely. The library would > depend on > >> > MemoryAllocationLib again, but also would have an > >> optimisation by > >> > choosing stack vs pool based on ElementSize. > >> > Usually I would prefer 2), as it is less prone to caller > >> errors, but > >> > considering the low-level nature of edk2, I can totally see > the > >> > point to > >> > allow the caller to control whether there are dynamic > memory > >> > allocations > >> > made or not as possible with 1). 2) could technically also = be > a > >> > wrapper > >> > for 1) if you want granular control and convenience/safety > >> (why not > >> > actually?). > >> > Both approaches have the advantage that it is crystal-clear > >> what the > >> > caller's job is - to always or to never allocate the buffer= . > >> > Best regards, > >> > Marvin > >> > On 26/09/2021 04:24, Ni, Ray wrote: > >> > > > >> > > Liming, > >> > > > >> > > The purpose of the optional BufferOneElement is to ease > >> consumer=E2=80=99s > >> > > life assuming most of the time the element size should be > >> > smaller than > >> > > 256. > >> > > > >> > > Are you saying that it=E2=80=99s a bit hard to calculate = the actual > >> > value of > >> > > sizeof (Element) when writing code? > >> > > > >> > > I recommend consumer always allocates memory if it=E2=80= =99s > hard > >> to judge > >> > > sizeof (Element) < 256. > >> > > > >> > > Searching edk2 code, I can find below code using > >> PerformQuickSort(): > >> > > > >> > > 1. ShellPkg/UefiShellCommandLib: It=E2=80=99s sorting ar= ray of > >> > > (EFI_DEVICE_PATH_PROTOCOL *). > >> > > 2. UefiCpuPkg/CpuCacheInfoLib: It=E2=80=99s sorting arra= y of > >> > CPU_CACHE_INFO. > >> > > 3. MinPlatformPkg/AcpiTables: It=E2=80=99s sorting array= of > >> > EFI_CPU_ID_ORDER_MAP. > >> > > 4. MdeModulePkg/UefiBootManagerLib: It=E2=80=99s sorting > array of > >> > > EFI_BOOT_MANAGER_LOAD_OPTION. > >> > > 5. MdeModulePkg/CapsuleApp: It=E2=80=99s sorting array o= f > >> (EFI_FILE_INFO *) > >> > > 6. CryptoPkg/CrtWrapper: It=E2=80=99s sorting array of > (unknown > >> type). > >> > > 7. RedfishPkg/RedfishCrtLib: It=E2=80=99s sorting array = of > >> (unknown type). > >> > > > >> > > For 1~5, it=E2=80=99s easy to know that the size of the e= lement is > >> smaller > >> > > than 256. The AllocatePool() can be skipped. > >> > > > >> > > Thanks, > >> > > > >> > > Ray > >> > > > >> > > *From:*gaoliming > >> > > *Sent:* Sunday, September 26, 2021 10:01 AM > >> > > *To:* Ni, Ray ; devel@edk2.groups.io; > >> Chan, Amy > >> > > ; 'Andrew Fish' > > >> > > *Cc:* Kinney, Michael D ; > >> 'Gao, Liming' > >> > > ; Liu, Zhiguang > >> ; > >> > Wang, > >> > > Jian J ; Gao, Zhichao > >> > >> > > *Subject:* =E5=9B=9E=E5=A4=8D: [edk2-devel] RFC: Add > BaseLib/QuickSort in > >> MdePkg > >> > > > >> > > Ray: > >> > > > >> > > I may suggest to always require BufferOneElement. The > >> consumer code > >> > > may not know ElementSize. To avoid the error, the > consumer > >> must > >> > > allocate buffer for BufferOneElement. If so, > >> BufferOneElement is > >> > the > >> > > required parameter. > >> > > > >> > > Thanks > >> > > > >> > > Liming > >> > > > >> > > *=E5=8F=91=E4=BB=B6=E4=BA=BA**:*Ni, Ray >> > > >> > > *=E5=8F=91=E9=80=81=E6=97=B6=E9=97=B4:* 2021=E5=B9=B49=E6= =9C=8824=E6=97=A5 11:53 > >> > > *=E6=94=B6=E4=BB=B6=E4=BA=BA:* devel@edk2.groups.io > ; > >> Ni, > >> > Ray > >> > > >; Chan, > Amy > >> > > >; > gaoliming > >> > > >; > >> > 'Andrew > >> > > Fish' > > >> > > *=E6=8A=84=E9=80=81:* Kinney, Michael D >> > > >; 'Gao, Liming' > >> > > >; > Liu, > >> Zhiguang > >> > > >; > >> Wang, > >> > Jian J > >> > > >; > Gao, > >> > Zhichao > >> > > > > >> > > *=E4=B8=BB=E9=A2=98:* RE: [edk2-devel] RFC: Add BaseLib/Q= uickSort in > MdePkg > >> > > > >> > > More details on new QuickSort() API: > >> > > > >> > > The new API needs to carry additional parameter > >> =E2=80=9CBufferOneElement=E2=80=9D. > >> > > > >> > > This parameter gives QuickSort() the temporary buffer for > >> > swapping in > >> > > sorting. > >> > > > >> > > It=E2=80=99s to avoid BaseLib depends on MemoryAllocation= Lib. > >> > > > >> > > =E2=80=A6 > >> > > > >> > > @param [in] BufferOneElement When ElementSize > > 256, caller > >> > needs to > >> > > provide a buffer whose size > >> > > equals to > ElementSize. It=E2=80=99s > >> used by > >> > > QuickSort() for swapping in sorting. > >> > > > >> > > When ElementSize <=3D 256, QuickSort() uses a local stack > >> 256-byte > >> > buffer. > >> > > > >> > > @retval EFI_INVALID_PARAMETER When (ElementSize > > 256) and > >> > > (BufferOneElement =3D=3D NULL). > >> > > > >> > > =E2=80=A6 > >> > > > >> > > VOID > >> > > > >> > > EFIAPI > >> > > > >> > > QuickSort ( > >> > > > >> > > IN OUT > VOID *BufferToSort, > >> > > > >> > > IN CONST UINTN ElementCount, > >> > > > >> > > IN CONST UINTN ElementSize, > >> > > > >> > > IN SORT_COMPARE CompareFunction, > >> > > > >> > > IN VOID *BufferOneElement OPTIONAL > >> > > > >> > > ); > >> > > > >> > > Any comments? > >> > > > >> > > *From:*devel@edk2.groups.io > > >> > > > > *On > >> Behalf Of > >> > > *Ni, Ray > >> > > *Sent:* Wednesday, September 22, 2021 2:04 PM > >> > > *To:* Chan, Amy >> >; > >> > > gaoliming >> > > >; 'Andrew Fish' > >> >> > > >; 'edk2-devel-groups-io' > >> > > > > >> > > *Cc:* Kinney, Michael D >> > > >; 'Gao, Liming' > >> > > >; > Liu, > >> Zhiguang > >> > > >; > >> Wang, > >> > Jian J > >> > > >; > Gao, > >> > Zhichao > >> > > > > >> > > *Subject:* Re: [edk2-devel] RFC: Add BaseLib/QuickSort > in > >> MdePkg > >> > > > >> > > I don=E2=80=99t see objection so far. > >> > > > >> > > So, the final solution is: > >> > > > >> > > 1. Add QuickSort() API to BaseLib in MdePkg. > >> > > 2. Update existing MdeModulePkg/SortLib to use > >> QuickSort() in the > >> > > implementation (proposed by Andrew Fish and > Liming Gao) > >> > > 3. Update UefiCpuPkg to use QuickSortLib to remove > improper > >> > > dependency on MdeModulePkg > >> > > > >> > > Thanks, > >> > > > >> > > Ray > >> > > > >> > > *From:*Ni, Ray > >> > > *Sent:* Thursday, September 16, 2021 10:48 AM > >> > > *To:* Chan, Amy >> >; > >> > > gaoliming >> > > >; 'Andrew Fish' > >> >> > > >; 'edk2-devel-groups-io' > >> > > > > >> > > *Cc:* Kinney, Michael D >> > > >; 'Gao, Liming' > >> > > >; > Liu, > >> Zhiguang > >> > > >; > >> Wang, > >> > Jian J > >> > > >; > Gao, > >> > Zhichao > >> > > > > >> > > *Subject:* RE: [edk2-devel] RFC: Add BaseLib/QuickSort > in > >> MdePkg > >> > > > >> > > Amy, > >> > > > >> > > No. We only Add QuickSort() function API into BaseLib.h. > >> > > > >> > > *From:*Chan, Amy >> > > >> > > *Sent:* Thursday, September 16, 2021 10:46 AM > >> > > *To:* gaoliming >> > > >; 'Andrew Fish' > >> >> > > >; 'edk2-devel-groups-io' > >> > > > > >> > > *Cc:* Ni, Ray >> >; Kinney, > >> > > Michael D >> > > >; 'Gao, Liming' > >> > > >; > Liu, > >> Zhiguang > >> > > >; > >> Wang, > >> > Jian J > >> > > >; > Gao, > >> > Zhichao > >> > > > > >> > > *Subject:* RE: [edk2-devel] RFC: Add BaseLib/QuickSort > in > >> MdePkg > >> > > > >> > > Just to double confirm, will we have the null instance of > >> > QuickSort in > >> > > MdePkg? > >> > > > >> > > Regards, > >> > > > >> > > Amy > >> > > > >> > > *From:*gaoliming >> > > > > >> > > *Sent:* Thursday, September 16, 2021 10:23 AM > >> > > *To:* 'Andrew Fish' >> >; > >> > > 'edk2-devel-groups-io' >> > > > > >> > > *Cc:* Ni, Ray >> >; Kinney, > >> > > Michael D >> > > >; 'Gao, Liming' > >> > > >; > Liu, > >> Zhiguang > >> > > >; > >> Wang, > >> > Jian J > >> > > >; > Gao, > >> > Zhichao > >> > > >; > >> Chan, Amy > >> > > > > >> > > *Subject:* =E5=9B=9E=E5=A4=8D: [edk2-devel] RFC: Add > BaseLib/QuickSort in > >> MdePkg > >> > > > >> > > Andrew: > >> > > > >> > > Thanks for your suggestion. I think your idea is better. > >> We add new > >> > > QuickSort() API to BaseLib, and update SortLib library > >> instance to > >> > > consume BaseLib QuickSort() API. This way has no change > in > >> current > >> > > SortLib library class. It is the compatible solution. > >> > > > >> > > Thanks > >> > > > >> > > Liming > >> > > > >> > > *=E5=8F=91=E4=BB=B6=E4=BA=BA**:*Andrew Fish >> > > >> > > *=E5=8F=91=E9=80=81=E6=97=B6=E9=97=B4:* 2021=E5=B9=B49=E6= =9C=8816=E6=97=A5 10:13 > >> > > *=E6=94=B6=E4=BB=B6=E4=BA=BA:* edk2-devel-groups-io >> > > >; Liming Gao > >> > >> > > > > >> > > *=E6=8A=84=E9=80=81:* Ni, Ray >> >; Mike > >> > > Kinney >> > > >; Gao, Liming > >> > > >; > Liu, > >> Zhiguang > >> > > >; > >> Wang, > >> > Jian J > >> > > >; > Gao, > >> > Zhichao > >> > > >; > >> Chan, Amy > >> > > > > >> > > *=E4=B8=BB=E9=A2=98:* Re: [edk2-devel] RFC: Add BaseLib/Q= uickSort in > MdePkg > >> > > > >> > > On Sep 15, 2021, at 6:26 PM, gaoliming > >> >> > > > wrote: > >> > > > >> > > Ray: > >> > > > >> > > SortLib has been added since 2015. I would > suggest to > >> still keep > >> > > this library class. To resolve the package > dependency, my > >> > proposal > >> > > is to move the library class header file SortLib.h > from > >> > > MdeModulePkg to MdePkg, and still keep the > library > >> instance in > >> > > MdeModulePkg. This proposal has no impact on > the existing > >> > platform. > >> > > > >> > > If we add QuickSort() API to the BaseLib can we not just > >> port the > >> > > existing MdeModulePkg/SortLib to use QuickSort() in the > >> > > implementation? Or is there some other way to add the > new > >> thing > >> > in a > >> > > backward compatible way. > >> > > > >> > > Thanks, > >> > > > >> > > Andrew Fish > >> > > > >> > > Thanks > >> > > > >> > > Liming > >> > > > >> > > *=E5=8F=91=E4=BB=B6=E4=BA=BA**:*devel@edk2.groups.io > >> > > >> > > >*=E4=BB=A3=E8=A1=A8***Ni, R= ay > >> > > *=E5=8F=91=E9=80=81=E6=97=B6=E9=97=B4:*2021=E5=B9=B49= =E6=9C=8814=E6=97=A514:15 > >> > > *=E6=94=B6=E4=BB=B6=E4=BA=BA:*Kinney, Michael D > >> > > >; Gao, Liming > >> > > >; Liu, > >> > > Zhiguang >> > >; > >> > > Wang, Jian J >> > > >; Gao, Zhichao > >> > > > > >> > > *=E6=8A=84=E9=80=81:*devel@edk2.groups.io > ; > >> > Chan, Amy > >> > > > > >> > > *=E4=B8=BB=E9=A2=98:*[edk2-devel] RFC: Add BaseLib/Qu= ickSort > in MdePkg > >> > > > >> > > Hi package maintainers of MdePkg, > MdeModulePkg and > >> ShellPkg, > >> > > community, > >> > > > >> > > A commit (UefiCpuPkg/CpuCacheInfoLib: Sort > >> CpuCacheInfo array > >> > > > >> > > >> > 5ab3516f3abec6>) > >> > > to UefiCpuPkg let > >> > > UefiCpuPkg depend on MdeModulePkg because > the SortLib > >> class and > >> > > instances are all in MdeModulePkg. > >> > > > >> > > UefiCpuPkg depending on MdeModulePkg breaks > the rule that > >> > > =E2=80=9CUefiCpuPkg should ONLY depend on MdePkg=E2= =80=9D. > >> > > > >> > > To address this issue, there are two approaches: > >> > > > >> > > 1. Duplicate the sort logic in UefiCpuPkg to not > >> depend on > >> > > MdeModulePkg/SortLib > >> > > 2. Add QuickSort() API to BaseLib in MdePkg. > >> > > > >> > > Approach #2 (MdePkg/BaseLib/QuickSort) makes > more > >> sense because > >> > > quick sort is a standard algorithm. > >> > > > >> > > We encourage consumers to update their code to > use the > >> quick > >> > sort > >> > > in MdePkg and gradually deprecate today=E2=80=99s > >> MdeModulePkg/SortLib. > >> > > > >> > > If you don=E2=80=99t have concerns, I plan to: > >> > > > >> > > 1. =E2=80=9CAdd QuickSort() to BaseLib=E2=80=9D and = update all > existing > >> > consumers > >> > > to use this API instead. > >> > > > >> > > VOID > >> > > > >> > > EFIAPI > >> > > > >> > > QuickSort ( > >> > > > >> > > IN OUT VOID *BufferT > oSort, > >> > > > >> > > IN CONST UINTN Count, > >> > > > >> > > IN CONST UINTN ElementSize, > >> > > > >> > > IN SORT_COMPARE Compare > Function > >> > > > >> > > ); > >> > > > >> > > 2. =E2=80=9CAdd new ShellPkg/SortCompareLib=E2=80=9D > >> > > > >> > > Background: ShellPkg requires to sort > >> devicepath/string so 3 > >> > APIs > >> > > in UefiSortLib (DevicePathCompare, > StringNoCaseCompare, > >> > > StringCompare) are provided for Shell usage. we > can > >> move the 3 > >> > > APIs to the SortCompareLib and update Shell code > to use > >> > > BaseLib/QuickSort directly, with the sort compare > >> function from > >> > > SortCompareLib. > >> > > > >> > > Any concerns? > >> > > > >> > > Thanks, > >> > > > >> > > Ray > >> > > > >> > > > >> > > >> > > >> > >> >=20 >=20 >=20 >=20 >=20 >=20 >=20 >=20 > -- >=20 > Brian >=20 > -------------------------------------------------------------------- >=20 > "Remember that ignorance is expensive." > -- From LLIB >=20