<html><head><meta http-equiv="content-type" content="text/html; charset=UTF-8"><style>body { line-height: 1.5; }blockquote { margin-top: 0px; margin-bottom: 0px; margin-left: 0.5em; }p { margin-top: 0px; margin-bottom: 0px; }div.FoxDiv20210927165943755050 { }body { font-size: 14px; font-family: "Microsoft YaHei UI"; color: rgb(0, 0, 0); line-height: 1.5; }</style></head><body>
 
<div style="FONT-FAMILY: Tahoma">In fact, my concern is that making BaseLib's APIs depend on PeiService & Boot Services is not good.  </div><div style="FONT-FAMILY: Tahoma"><br></div><div style="FONT-FAMILY: Tahoma"><span></span>For a instance,  AP functions by MP service are not allowed to invoke any PeriServices and Boot Services.  And exception handlers cannot invoke those services either.</div>
<div><br></div>
<hr style="width: 210px; height: 1px;" align="left" color="#b5c4df" size="1">
<div><span><div style="MARGIN: 10px; FONT-FAMILY: verdana; FONT-SIZE: 10pt"><div>Jeff</div><div>fanjianfeng@byosoft.com.cn</div></div></span></div><blockquote style="margin-top: 0px; margin-bottom: 0px; margin-left: 0.5em; margin-right: inherit;"><div> </div><div style="border:none;border-top:solid #B5C4DF 1.0pt;padding:3.0pt 0cm 0cm 0cm"><div style="PADDING-RIGHT: 8px; PADDING-LEFT: 8px; FONT-SIZE: 12px;FONT-FAMILY:tahoma;COLOR:#000000; BACKGROUND: #efefef; PADDING-BOTTOM: 8px; PADDING-TOP: 8px"><div><b>From:</b> <a href="mailto:fanjianfeng@byosoft.com.cn">Jeff Fan</a></div><div><b>Date:</b> 2021-09-27 16:50</div><div><b>To:</b> <a href="mailto:devel@edk2.groups.io">devel@edk2.groups.io</a>; <a href="mailto:mhaeuser@posteo.de">mhaeuser</a>; <a href="mailto:ray.ni@intel.com">Ni, Ray</a>; <a href="mailto:gaoliming@byosoft.com.cn">'gaoliming'</a>; <a href="mailto:amy.chan@intel.com">Chan, Amy</a>; <a href="mailto:afish@apple.com">'Andrew Fish'</a></div><div><b>CC:</b> <a href="mailto:michael.d.kinney@intel.com">Kinney, Michael D</a>; <a href="mailto:liming.gao@intel.com">'Gao, Liming'</a>; <a href="mailto:zhiguang.liu@intel.com">Liu, Zhiguang</a>; <a href="mailto:jian.j.wang@intel.com">Wang, Jian J</a>; <a href="mailto:zhichao.gao@intel.com">Gao, Zhichao</a></div><div><b>Subject:</b> Re: [edk2-devel] RFC: Add BaseLib/QuickSort in MdePkg</div></div></div><div><div class="FoxDiv20210927165943755050">
 
<div style="FONT-FAMILY: Tahoma">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 code.</div><div style="FONT-FAMILY: Tahoma"><span style="font-size: 11pt; background-color: transparent;">Only when the existing code wants to remove the dependency on MdeModuelPkg, they could migrate to the new API in baselib.</span></div><div style="FONT-FAMILY: Tahoma"><br></div><div style="FONT-FAMILY: Tahoma"><span style="background-color: transparent; font-size: 11pt;">I agree with one split-API desgin what you mentioned. </span></div><div style="FONT-FAMILY: Tahoma">But my point is to define one API in baselib and to make the baselib not depend on memory allocation.  <span style="background-color: transparent; font-size: 11pt;">And another wrapper API could be designed to be placed in any other class.</span></div>
<div><br></div>
<hr style="width: 210px; height: 1px;" align="left" color="#b5c4df" size="1">
<div><span><div style="MARGIN: 10px; FONT-FAMILY: verdana; FONT-SIZE: 10pt"><div>Jeff</div><div>fanjianfeng@byosoft.com.cn</div></div></span></div><blockquote style="margin-top: 0px; margin-bottom: 0px; margin-left: 0.5em; margin-right: inherit;"><div> </div><div style="border:none;border-top:solid #B5C4DF 1.0pt;padding:3.0pt 0cm 0cm 0cm"><div style="PADDING-RIGHT: 8px; PADDING-LEFT: 8px; FONT-SIZE: 12px;FONT-FAMILY:tahoma;COLOR:#000000; BACKGROUND: #efefef; PADDING-BOTTOM: 8px; PADDING-TOP: 8px"><div><b>From:</b> <a href="mailto:mhaeuser@posteo.de">Marvin Häuser</a></div><div><b>Date:</b> 2021-09-27 16:14</div><div><b>To:</b> <a href="mailto:fanjianfeng@byosoft.com.cn">fanjianfeng@byosoft.com.cn</a>; <a href="mailto:devel@edk2.groups.io">devel@edk2.groups.io</a>; <a href="mailto:ray.ni@intel.com">Ni, Ray</a>; <a href="mailto:gaoliming@byosoft.com.cn">'gaoliming'</a>; <a href="mailto:amy.chan@intel.com">Chan, Amy</a>; <a href="mailto:afish@apple.com">'Andrew Fish'</a></div><div><b>CC:</b> <a href="mailto:michael.d.kinney@intel.com">Kinney, Michael D</a>; <a href="mailto:liming.gao@intel.com">'Gao, Liming'</a>; <a href="mailto:zhiguang.liu@intel.com">Liu, Zhiguang</a>; <a href="mailto:jian.j.wang@intel.com">Wang, Jian J</a>; <a href="mailto:zhichao.gao@intel.com">Gao, Zhichao</a></div><div><b>Subject:</b> Re: [edk2-devel] RFC: Add BaseLib/QuickSort in MdePkg</div></div></div><div><div>On 27/09/2021 02:45, fanjianfeng@byosoft.com.cn wrote:</div>
<div>> Making baselib implementation depend on MemoryAllocationLib </div>
<div>> (indirectly on Pei Service and gBS), it may prevent </div>
<div>> this base API using at some seneraio. i don't think it's better.</div>
<div> </div>
<div>That is why I asked about a split-API scenario, one of which does not </div>
<div>depend on dynamic memory allocation (SetVariable-like) and one does </div>
<div>(wrapper-like).</div>
<div> </div>
<div>> Add this parameter and make this parameter is optional,</div>
<div>> 1, when NULL, use the local 256 bytes stack</div>
<div>> 2, if 256 bytes stack is not enough, return RETURE_BUFFER_TOO_SAMLL,</div>
<div>> 3, caller check the return status, to do nothing or to allocate enough </div>
<div>> buffer for retry</div>
<div>></div>
<div>> This is just like SetVariable()'s implementation.</div>
<div> </div>
<div>Yes, and because that is SetVariable's implementation, we have library </div>
<div>functions to make it less error-prone and more convenient [1]. As a </div>
<div>matter of fact, we have a (semi-lax) policy in our codebases to avoid </div>
<div>such functions like the plague and use those library wrappers where-ever </div>
<div>it can make sense. The only super-rare exceptions I can think of are </div>
<div>when we know the size of the element ahead of time. Also SetVariable has </div>
<div>no arbitrary constraint on when it may work the first time, and there is </div>
<div>code that will fail when the first return is not EFI_BUFFER_TOO_SMALL.</div>
<div> </div>
<div>This solution honestly yields even more problems, because it introduces </div>
<div>a Status return which was not there before. For common code safety and </div>
<div>quality policy, this requires the value *must* be retrieved and checked </div>
<div>in some fashion. So all callers, no matter the prior knowledge of the </div>
<div>element size, now need either a runtime check and handling for a status </div>
<div>that they (may) know can never be returned, or ASSERTs if the function </div>
<div>spec really imposes the arbitrary 256 Bytes constraint. Latter doesn't </div>
<div>really work I think. What if someone wants to sort in SEC and noticed </div>
<div>they only have 64 Bytes on the stack to work with, realistically? Any </div>
<div>code depending on the 256 Byte constraint, passing NULL and not doing </div>
<div>additional handling, would seize to work. Former is too complicated, see </div>
<div>wrappers. In my opinion, the memory must *either* be fully managed by </div>
<div>the caller *or* the callee (and you may have both in separate functions, </div>
<div>as I suggested), but not sometimes here, sometimes there.</div>
<div> </div>
<div>Best regards,</div>
<div>Marvin</div>
<div> </div>
<div> </div>
<div>[1] </div>
<div>https://github.com/tianocore/edk2/blob/46b4606ba23498d3d0e66b53e498eb3d5d592586/MdePkg/Library/UefiLib/UefiLib.c#L1309-L1360</div>
<div> </div>
<div>></div>
<div>> ------------------------------------------------------------------------</div>
<div>> Jeff</div>
<div>> fanjianfeng@byosoft.com.cn</div>
<div>></div>
<div>>     *From:* Marvin Häuser <mailto:mhaeuser@posteo.de></div>
<div>>     *Date:* 2021-09-26 19:20</div>
<div>>     *To:* devel <mailto:devel@edk2.groups.io>; ray.ni</div>
<div>>     <mailto:ray.ni@intel.com>; gaoliming</div>
<div>>     <mailto:gaoliming@byosoft.com.cn>; Chan, Amy</div>
<div>>     <mailto:amy.chan@intel.com>; 'Andrew Fish' <mailto:afish@apple.com></div>
<div>>     *CC:* Kinney, Michael D <mailto:michael.d.kinney@intel.com>; 'Gao,</div>
<div>>     Liming' <mailto:liming.gao@intel.com>; Liu, Zhiguang</div>
<div>>     <mailto:zhiguang.liu@intel.com>; Wang, Jian J</div>
<div>>     <mailto:jian.j.wang@intel.com>; Gao, Zhichao</div>
<div>>     <mailto:zhichao.gao@intel.com></div>
<div>>     *Subject:* Re: [edk2-devel] RFC: Add BaseLib/QuickSort in MdePkg</div>
<div>>     Hey Ray,</div>
<div>>     In my opinion that spec is too complicated. For some cases it is</div>
<div>>     obvious, but I think the last anyone wants to see is a</div>
<div>>     (STATIC_)ASSERT</div>
<div>>     before most QuickSort calls to ensure the element size *really* is <=</div>
<div>>     256 Bytes. In my opinion, there are two roads:</div>
<div>>     1) Make the parameter required (I think this is what Liming</div>
<div>>     suggested).</div>
<div>>     The caller would always need to provide said buffer, and it can do</div>
<div>>     as it</div>
<div>>     sees fit - on the stack, in a pool, in a page, who knows.</div>
<div>>     2) Remove the parameter entirely. The library would depend on</div>
<div>>     MemoryAllocationLib again, but also would have an optimisation by</div>
<div>>     choosing stack vs pool based on ElementSize.</div>
<div>>     Usually I would prefer 2), as it is less prone to caller errors, but</div>
<div>>     considering the low-level nature of edk2, I can totally see the</div>
<div>>     point to</div>
<div>>     allow the caller to control whether there are dynamic memory</div>
<div>>     allocations</div>
<div>>     made or not as possible with 1). 2) could technically also be a</div>
<div>>     wrapper</div>
<div>>     for 1) if you want granular control and convenience/safety (why not</div>
<div>>     actually?).</div>
<div>>     Both approaches have the advantage that it is crystal-clear what the</div>
<div>>     caller's job is - to always or to never allocate the buffer.</div>
<div>>     Best regards,</div>
<div>>     Marvin</div>
<div>>     On 26/09/2021 04:24, Ni, Ray wrote:</div>
<div>>     ></div>
<div>>     > Liming,</div>
<div>>     ></div>
<div>>     > The purpose of the optional BufferOneElement is to ease consumer’s</div>
<div>>     > life assuming most of the time the element size should be</div>
<div>>     smaller than</div>
<div>>     > 256.</div>
<div>>     ></div>
<div>>     > Are you saying that it’s a bit hard to calculate the actual</div>
<div>>     value of</div>
<div>>     > sizeof (Element) when writing code?</div>
<div>>     ></div>
<div>>     > I recommend consumer always allocates memory if it’s hard to judge</div>
<div>>     > sizeof (Element) < 256.</div>
<div>>     ></div>
<div>>     > Searching edk2 code, I can find below code using PerformQuickSort():</div>
<div>>     ></div>
<div>>     >  1. ShellPkg/UefiShellCommandLib: It’s sorting array of</div>
<div>>     >     (EFI_DEVICE_PATH_PROTOCOL *).</div>
<div>>     >  2. UefiCpuPkg/CpuCacheInfoLib: It’s sorting array of</div>
<div>>      CPU_CACHE_INFO.</div>
<div>>     >  3. MinPlatformPkg/AcpiTables: It’s sorting array of</div>
<div>>     EFI_CPU_ID_ORDER_MAP.</div>
<div>>     >  4. MdeModulePkg/UefiBootManagerLib: It’s sorting array of</div>
<div>>     >     EFI_BOOT_MANAGER_LOAD_OPTION.</div>
<div>>     >  5. MdeModulePkg/CapsuleApp: It’s sorting array of (EFI_FILE_INFO *)</div>
<div>>     >  6. CryptoPkg/CrtWrapper: It’s sorting array of (unknown type).</div>
<div>>     >  7. RedfishPkg/RedfishCrtLib: It’s sorting array of (unknown type).</div>
<div>>     ></div>
<div>>     > For 1~5, it’s easy to know that the size of the element is smaller</div>
<div>>     > than 256. The AllocatePool() can be skipped.</div>
<div>>     ></div>
<div>>     > Thanks,</div>
<div>>     ></div>
<div>>     > Ray</div>
<div>>     ></div>
<div>>     > *From:*gaoliming <gaoliming@byosoft.com.cn></div>
<div>>     > *Sent:* Sunday, September 26, 2021 10:01 AM</div>
<div>>     > *To:* Ni, Ray <ray.ni@intel.com>; devel@edk2.groups.io; Chan, Amy</div>
<div>>     > <amy.chan@intel.com>; 'Andrew Fish' <afish@apple.com></div>
<div>>     > *Cc:* Kinney, Michael D <michael.d.kinney@intel.com>; 'Gao, Liming'</div>
<div>>     > <liming.gao@intel.com>; Liu, Zhiguang <zhiguang.liu@intel.com>;</div>
<div>>     Wang,</div>
<div>>     > Jian J <jian.j.wang@intel.com>; Gao, Zhichao <zhichao.gao@intel.com></div>
<div>>     > *Subject:* 回复: [edk2-devel] RFC: Add BaseLib/QuickSort in MdePkg</div>
<div>>     ></div>
<div>>     > Ray:</div>
<div>>     ></div>
<div>>     > I may suggest to always require BufferOneElement. The consumer code</div>
<div>>     > may not know ElementSize. To avoid the error, the consumer must</div>
<div>>     > allocate buffer for BufferOneElement. If so, BufferOneElement is</div>
<div>>     the</div>
<div>>     > required parameter.</div>
<div>>     ></div>
<div>>     > Thanks</div>
<div>>     ></div>
<div>>     > Liming</div>
<div>>     ></div>
<div>>     > *发件人**:*Ni, Ray <ray.ni@intel.com <mailto:ray.ni@intel.com>></div>
<div>>     > *发送时间:* 2021年9月24日 11:53</div>
<div>>     > *收件人:* devel@edk2.groups.io <mailto:devel@edk2.groups.io>; Ni,</div>
<div>>     Ray</div>
<div>>     > <ray.ni@intel.com <mailto:ray.ni@intel.com>>; Chan, Amy</div>
<div>>     > <amy.chan@intel.com <mailto:amy.chan@intel.com>>; gaoliming</div>
<div>>     > <gaoliming@byosoft.com.cn <mailto:gaoliming@byosoft.com.cn>>;</div>
<div>>     'Andrew</div>
<div>>     > Fish' <afish@apple.com <mailto:afish@apple.com>></div>
<div>>     > *抄送:* Kinney, Michael D <michael.d.kinney@intel.com</div>
<div>>     > <mailto:michael.d.kinney@intel.com>>; 'Gao, Liming'</div>
<div>>     > <liming.gao@intel.com <mailto:liming.gao@intel.com>>; Liu, Zhiguang</div>
<div>>     > <zhiguang.liu@intel.com <mailto:zhiguang.liu@intel.com>>; Wang,</div>
<div>>     Jian J</div>
<div>>     > <jian.j.wang@intel.com <mailto:jian.j.wang@intel.com>>; Gao,</div>
<div>>     Zhichao</div>
<div>>     > <zhichao.gao@intel.com <mailto:zhichao.gao@intel.com>></div>
<div>>     > *主题:* RE: [edk2-devel] RFC: Add BaseLib/QuickSort in MdePkg</div>
<div>>     ></div>
<div>>     > More details on new QuickSort() API:</div>
<div>>     ></div>
<div>>     > The new API needs to carry additional parameter “BufferOneElement”.</div>
<div>>     ></div>
<div>>     > This parameter gives QuickSort() the temporary buffer for</div>
<div>>     swapping in</div>
<div>>     > sorting.</div>
<div>>     ></div>
<div>>     > It’s to avoid BaseLib depends on MemoryAllocationLib.</div>
<div>>     ></div>
<div>>     > …</div>
<div>>     ></div>
<div>>     > @param [in] BufferOneElement  When ElementSize > 256, caller</div>
<div>>     needs to</div>
<div>>     > provide a buffer whose size</div>
<div>>     >                               equals to ElementSize. It’s used by</div>
<div>>     > QuickSort() for swapping in sorting.</div>
<div>>     ></div>
<div>>     > When ElementSize <= 256, QuickSort() uses a local stack 256-byte</div>
<div>>     buffer.</div>
<div>>     ></div>
<div>>     > @retval EFI_INVALID_PARAMETER When (ElementSize > 256) and</div>
<div>>     > (BufferOneElement == NULL).</div>
<div>>     ></div>
<div>>     > …</div>
<div>>     ></div>
<div>>     > VOID</div>
<div>>     ></div>
<div>>     > EFIAPI</div>
<div>>     ></div>
<div>>     > QuickSort (</div>
<div>>     ></div>
<div>>     >   IN OUT VOID                         *BufferToSort,</div>
<div>>     ></div>
<div>>     >   IN CONST UINTN ElementCount,</div>
<div>>     ></div>
<div>>     >   IN CONST UINTN ElementSize,</div>
<div>>     ></div>
<div>>     >   IN       SORT_COMPARE CompareFunction,</div>
<div>>     ></div>
<div>>     > IN VOID                         *BufferOneElement OPTIONAL</div>
<div>>     ></div>
<div>>     > );</div>
<div>>     ></div>
<div>>     > Any comments?</div>
<div>>     ></div>
<div>>     > *From:*devel@edk2.groups.io <mailto:devel@edk2.groups.io></div>
<div>>     > <devel@edk2.groups.io <mailto:devel@edk2.groups.io>> *On Behalf Of</div>
<div>>     > *Ni, Ray</div>
<div>>     > *Sent:* Wednesday, September 22, 2021 2:04 PM</div>
<div>>     > *To:* Chan, Amy <amy.chan@intel.com <mailto:amy.chan@intel.com>>;</div>
<div>>     > gaoliming <gaoliming@byosoft.com.cn</div>
<div>>     > <mailto:gaoliming@byosoft.com.cn>>; 'Andrew Fish' <afish@apple.com</div>
<div>>     > <mailto:afish@apple.com>>; 'edk2-devel-groups-io'</div>
<div>>     > <devel@edk2.groups.io <mailto:devel@edk2.groups.io>></div>
<div>>     > *Cc:* Kinney, Michael D <michael.d.kinney@intel.com</div>
<div>>     > <mailto:michael.d.kinney@intel.com>>; 'Gao, Liming'</div>
<div>>     > <liming.gao@intel.com <mailto:liming.gao@intel.com>>; Liu, Zhiguang</div>
<div>>     > <zhiguang.liu@intel.com <mailto:zhiguang.liu@intel.com>>; Wang,</div>
<div>>     Jian J</div>
<div>>     > <jian.j.wang@intel.com <mailto:jian.j.wang@intel.com>>; Gao,</div>
<div>>     Zhichao</div>
<div>>     > <zhichao.gao@intel.com <mailto:zhichao.gao@intel.com>></div>
<div>>     > *Subject:* Re: [edk2-devel] RFC: Add BaseLib/QuickSort in MdePkg</div>
<div>>     ></div>
<div>>     > I don’t see objection so far.</div>
<div>>     ></div>
<div>>     > So, the final solution is:</div>
<div>>     ></div>
<div>>     >  1. Add QuickSort() API to BaseLib in MdePkg.</div>
<div>>     >  2. Update existing MdeModulePkg/SortLib to use QuickSort() in the</div>
<div>>     >     implementation (proposed by Andrew Fish and Liming Gao)</div>
<div>>     >  3. Update UefiCpuPkg to use QuickSortLib to remove improper</div>
<div>>     >     dependency on MdeModulePkg</div>
<div>>     ></div>
<div>>     > Thanks,</div>
<div>>     ></div>
<div>>     > Ray</div>
<div>>     ></div>
<div>>     > *From:*Ni, Ray</div>
<div>>     > *Sent:* Thursday, September 16, 2021 10:48 AM</div>
<div>>     > *To:* Chan, Amy <amy.chan@intel.com <mailto:amy.chan@intel.com>>;</div>
<div>>     > gaoliming <gaoliming@byosoft.com.cn</div>
<div>>     > <mailto:gaoliming@byosoft.com.cn>>; 'Andrew Fish' <afish@apple.com</div>
<div>>     > <mailto:afish@apple.com>>; 'edk2-devel-groups-io'</div>
<div>>     > <devel@edk2.groups.io <mailto:devel@edk2.groups.io>></div>
<div>>     > *Cc:* Kinney, Michael D <michael.d.kinney@intel.com</div>
<div>>     > <mailto:michael.d.kinney@intel.com>>; 'Gao, Liming'</div>
<div>>     > <liming.gao@intel.com <mailto:liming.gao@intel.com>>; Liu, Zhiguang</div>
<div>>     > <Zhiguang.Liu@intel.com <mailto:Zhiguang.Liu@intel.com>>; Wang,</div>
<div>>     Jian J</div>
<div>>     > <jian.j.wang@intel.com <mailto:jian.j.wang@intel.com>>; Gao,</div>
<div>>     Zhichao</div>
<div>>     > <zhichao.gao@intel.com <mailto:zhichao.gao@intel.com>></div>
<div>>     > *Subject:* RE: [edk2-devel] RFC: Add BaseLib/QuickSort in MdePkg</div>
<div>>     ></div>
<div>>     > Amy,</div>
<div>>     ></div>
<div>>     > No. We only Add QuickSort() function API into BaseLib.h.</div>
<div>>     ></div>
<div>>     > *From:*Chan, Amy <amy.chan@intel.com <mailto:amy.chan@intel.com>></div>
<div>>     > *Sent:* Thursday, September 16, 2021 10:46 AM</div>
<div>>     > *To:* gaoliming <gaoliming@byosoft.com.cn</div>
<div>>     > <mailto:gaoliming@byosoft.com.cn>>; 'Andrew Fish' <afish@apple.com</div>
<div>>     > <mailto:afish@apple.com>>; 'edk2-devel-groups-io'</div>
<div>>     > <devel@edk2.groups.io <mailto:devel@edk2.groups.io>></div>
<div>>     > *Cc:* Ni, Ray <ray.ni@intel.com <mailto:ray.ni@intel.com>>; Kinney,</div>
<div>>     > Michael D <michael.d.kinney@intel.com</div>
<div>>     > <mailto:michael.d.kinney@intel.com>>; 'Gao, Liming'</div>
<div>>     > <liming.gao@intel.com <mailto:liming.gao@intel.com>>; Liu, Zhiguang</div>
<div>>     > <zhiguang.liu@intel.com <mailto:zhiguang.liu@intel.com>>; Wang,</div>
<div>>     Jian J</div>
<div>>     > <jian.j.wang@intel.com <mailto:jian.j.wang@intel.com>>; Gao,</div>
<div>>     Zhichao</div>
<div>>     > <zhichao.gao@intel.com <mailto:zhichao.gao@intel.com>></div>
<div>>     > *Subject:* RE: [edk2-devel] RFC: Add BaseLib/QuickSort in MdePkg</div>
<div>>     ></div>
<div>>     > Just to double confirm, will we have the null instance of</div>
<div>>     QuickSort in</div>
<div>>     > MdePkg?</div>
<div>>     ></div>
<div>>     > Regards,</div>
<div>>     ></div>
<div>>     > Amy</div>
<div>>     ></div>
<div>>     > *From:*gaoliming <gaoliming@byosoft.com.cn</div>
<div>>     > <mailto:gaoliming@byosoft.com.cn>></div>
<div>>     > *Sent:* Thursday, September 16, 2021 10:23 AM</div>
<div>>     > *To:* 'Andrew Fish' <afish@apple.com <mailto:afish@apple.com>>;</div>
<div>>     > 'edk2-devel-groups-io' <devel@edk2.groups.io</div>
<div>>     > <mailto:devel@edk2.groups.io>></div>
<div>>     > *Cc:* Ni, Ray <ray.ni@intel.com <mailto:ray.ni@intel.com>>; Kinney,</div>
<div>>     > Michael D <michael.d.kinney@intel.com</div>
<div>>     > <mailto:michael.d.kinney@intel.com>>; 'Gao, Liming'</div>
<div>>     > <liming.gao@intel.com <mailto:liming.gao@intel.com>>; Liu, Zhiguang</div>
<div>>     > <zhiguang.liu@intel.com <mailto:zhiguang.liu@intel.com>>; Wang,</div>
<div>>     Jian J</div>
<div>>     > <jian.j.wang@intel.com <mailto:jian.j.wang@intel.com>>; Gao,</div>
<div>>     Zhichao</div>
<div>>     > <zhichao.gao@intel.com <mailto:zhichao.gao@intel.com>>; Chan, Amy</div>
<div>>     > <amy.chan@intel.com <mailto:amy.chan@intel.com>></div>
<div>>     > *Subject:* 回复: [edk2-devel] RFC: Add BaseLib/QuickSort in MdePkg</div>
<div>>     ></div>
<div>>     > Andrew:</div>
<div>>     ></div>
<div>>     > Thanks for your suggestion. I think your idea is better. We add new</div>
<div>>     > QuickSort() API to BaseLib, and update SortLib library instance to</div>
<div>>     > consume BaseLib QuickSort() API. This way has no change in current</div>
<div>>     > SortLib library class. It is the compatible solution.</div>
<div>>     ></div>
<div>>     > Thanks</div>
<div>>     ></div>
<div>>     > Liming</div>
<div>>     ></div>
<div>>     > *发件人**:*Andrew Fish <afish@apple.com <mailto:afish@apple.com>></div>
<div>>     > *发送时间:* 2021年9月16日 10:13</div>
<div>>     > *收件人:* edk2-devel-groups-io <devel@edk2.groups.io</div>
<div>>     > <mailto:devel@edk2.groups.io>>; Liming Gao</div>
<div>>     <gaoliming@byosoft.com.cn</div>
<div>>     > <mailto:gaoliming@byosoft.com.cn>></div>
<div>>     > *抄送:* Ni, Ray <ray.ni@intel.com <mailto:ray.ni@intel.com>>; Mike</div>
<div>>     > Kinney <michael.d.kinney@intel.com</div>
<div>>     > <mailto:michael.d.kinney@intel.com>>; Gao, Liming</div>
<div>>     > <liming.gao@intel.com <mailto:liming.gao@intel.com>>; Liu, Zhiguang</div>
<div>>     > <zhiguang.liu@intel.com <mailto:zhiguang.liu@intel.com>>; Wang,</div>
<div>>     Jian J</div>
<div>>     > <jian.j.wang@intel.com <mailto:jian.j.wang@intel.com>>; Gao,</div>
<div>>     Zhichao</div>
<div>>     > <zhichao.gao@intel.com <mailto:zhichao.gao@intel.com>>; Chan, Amy</div>
<div>>     > <amy.chan@intel.com <mailto:amy.chan@intel.com>></div>
<div>>     > *主题:* Re: [edk2-devel] RFC: Add BaseLib/QuickSort in MdePkg</div>
<div>>     ></div>
<div>>     >     On Sep 15, 2021, at 6:26 PM, gaoliming <gaoliming@byosoft.com.cn</div>
<div>>     >     <mailto:gaoliming@byosoft.com.cn>> wrote:</div>
<div>>     ></div>
<div>>     >     Ray:</div>
<div>>     ></div>
<div>>     >     SortLib has been added since 2015. I would suggest to still keep</div>
<div>>     >     this library class. To resolve the package dependency, my</div>
<div>>     proposal</div>
<div>>     >     is to move the library class header file SortLib.h from</div>
<div>>     >     MdeModulePkg to MdePkg, and still keep the library instance in</div>
<div>>     >     MdeModulePkg. This proposal has no impact on the existing</div>
<div>>     platform.</div>
<div>>     ></div>
<div>>     > If we add QuickSort() API to the BaseLib can we not just port the</div>
<div>>     > existing MdeModulePkg/SortLib to use QuickSort() in the</div>
<div>>     > implementation? Or is there some other way to add the new thing</div>
<div>>     in a</div>
<div>>     > backward compatible way.</div>
<div>>     ></div>
<div>>     > Thanks,</div>
<div>>     ></div>
<div>>     > Andrew Fish</div>
<div>>     ></div>
<div>>     >     Thanks</div>
<div>>     ></div>
<div>>     >     Liming</div>
<div>>     ></div>
<div>>     >     *发件人**:*devel@edk2.groups.io</div>
<div>>     > <mailto:devel@edk2.groups.io><devel@edk2.groups.io</div>
<div>>     > <mailto:devel@edk2.groups.io>>*代表***Ni, Ray</div>
<div>>     >     *发送时间:*2021年9月14日14:15</div>
<div>>     >     *收件人:*Kinney, Michael D <michael.d.kinney@intel.com</div>
<div>>     >     <mailto:michael.d.kinney@intel.com>>; Gao, Liming</div>
<div>>     >     <liming.gao@intel.com <mailto:liming.gao@intel.com>>; Liu,</div>
<div>>     >     Zhiguang <zhiguang.liu@intel.com</div>
<div>>     <mailto:zhiguang.liu@intel.com>>;</div>
<div>>     >     Wang, Jian J <jian.j.wang@intel.com</div>
<div>>     >     <mailto:jian.j.wang@intel.com>>; Gao, Zhichao</div>
<div>>     >     <zhichao.gao@intel.com <mailto:zhichao.gao@intel.com>></div>
<div>>     >     *抄送:*devel@edk2.groups.io <mailto:devel@edk2.groups.io>;</div>
<div>>     Chan, Amy</div>
<div>>     >     <amy.chan@intel.com <mailto:amy.chan@intel.com>></div>
<div>>     >     *主题:*[edk2-devel] RFC: Add BaseLib/QuickSort in MdePkg</div>
<div>>     ></div>
<div>>     >     Hi package maintainers of MdePkg, MdeModulePkg and ShellPkg,</div>
<div>>     >     community,</div>
<div>>     ></div>
<div>>     >     A commit (UefiCpuPkg/CpuCacheInfoLib: Sort CpuCacheInfo array</div>
<div>>     ></div>
<div>>     <https://github.com/tianocore/edk2/commit/4de77ae9890d241271f543e9195ab3516f3abec6>)</div>
<div>>     >     to UefiCpuPkg let</div>
<div>>     >     UefiCpuPkg depend on MdeModulePkg because the SortLib class and</div>
<div>>     >     instances are all in MdeModulePkg.</div>
<div>>     ></div>
<div>>     >     UefiCpuPkg depending on MdeModulePkg breaks the rule that</div>
<div>>     >     “UefiCpuPkg should ONLY depend on MdePkg”.</div>
<div>>     ></div>
<div>>     >     To address this issue, there are two approaches:</div>
<div>>     ></div>
<div>>     >      1. Duplicate the sort logic in UefiCpuPkg to not depend on</div>
<div>>     >         MdeModulePkg/SortLib</div>
<div>>     >      2. Add QuickSort() API to BaseLib in MdePkg.</div>
<div>>     ></div>
<div>>     >     Approach #2 (MdePkg/BaseLib/QuickSort) makes more sense because</div>
<div>>     >     quick sort is a standard algorithm.</div>
<div>>     ></div>
<div>>     >     We encourage consumers to update their code to use the quick</div>
<div>>     sort</div>
<div>>     >     in MdePkg and gradually deprecate today’s MdeModulePkg/SortLib.</div>
<div>>     ></div>
<div>>     >     If you don’t have concerns, I plan to:</div>
<div>>     ></div>
<div>>     >      1. “Add QuickSort() to BaseLib” and update all existing</div>
<div>>     consumers</div>
<div>>     >         to use this API instead.</div>
<div>>     ></div>
<div>>     >     VOID</div>
<div>>     ></div>
<div>>     >     EFIAPI</div>
<div>>     ></div>
<div>>     >     QuickSort (</div>
<div>>     ></div>
<div>>     >       IN OUT VOID                   *BufferToSort,</div>
<div>>     ></div>
<div>>     >       IN CONST UINTN Count,</div>
<div>>     ></div>
<div>>     >       IN CONST UINTN ElementSize,</div>
<div>>     ></div>
<div>>     >   IN       SORT_COMPARE         CompareFunction</div>
<div>>     ></div>
<div>>     >       );</div>
<div>>     ></div>
<div>>     >      2. “Add new ShellPkg/SortCompareLib”</div>
<div>>     ></div>
<div>>     >     Background: ShellPkg requires to sort devicepath/string so 3</div>
<div>>     APIs</div>
<div>>     >     in UefiSortLib (DevicePathCompare, StringNoCaseCompare,</div>
<div>>     >     StringCompare) are provided for Shell usage. we can move the 3</div>
<div>>     >     APIs to the SortCompareLib and update Shell code to use</div>
<div>>     >     BaseLib/QuickSort directly, with the sort compare function from</div>
<div>>     >     SortCompareLib.</div>
<div>>     ></div>
<div>>     >     Any concerns?</div>
<div>>     ></div>
<div>>     >     Thanks,</div>
<div>>     ></div>
<div>>     >     Ray</div>
<div>>     ></div>
<div>>     ></div>
<div>>     </div>
<div>></div>
<div> </div>
<div> </div>
<div> </div>
<div></div>
<div> </div>
<div> </div>
<div> </div>
</div></blockquote>
  
</div></div></blockquote>
</body></html>


 <div width="1" style="color:white;clear:both">_._,_._,_</div> <hr>   Groups.io Links:<p>   You receive all messages sent to this group.    <p> <a target="_blank" href="https://edk2.groups.io/g/devel/message/81162">View/Reply Online (#81162)</a> |    |  <a target="_blank" href="https://groups.io/mt/85872410/1813853">Mute This Topic</a>  | <a href="https://edk2.groups.io/g/devel/post">New Topic</a><br>    <a href="https://edk2.groups.io/g/devel/editsub/1813853">Your Subscription</a> | <a href="mailto:devel+owner@edk2.groups.io">Contact Group Owner</a> |  <a href="https://edk2.groups.io/g/devel/unsub">Unsubscribe</a>  [edk2-devel-archive@redhat.com]<br> <div width="1" style="color:white;clear:both">_._,_._,_</div>