Re: [PATCHv2 4/5] efi_mem_sort: skip unnecessary compare

[Randolph Sapp <rs@ti.com>]

On Thu Apr 9, 2026 at 2:47 PM CDT, Heinrich Schuchardt wrote:
> Am 2. April 2026 22:50:37 MESZ schrieb rs@ti.com:
>>From: Randolph Sapp <rs@ti.com>
>>
>>I don't want to think about the possibility of this pointer containing a
>>reference to something from a previous iteration. At best it results in
>>a some arithmetic and a comparison that should usually be unnecessary.
>
> "a some arithmetic" is not understandable. Please, adjust the commit message.
>
>>
>>Signed-off-by: Randolph Sapp <rs@ti.com>
>>---
>> lib/efi_loader/efi_memory.c | 2 +-
>> 1 file changed, 1 insertion(+), 1 deletion(-)
>>
>>diff --git a/lib/efi_loader/efi_memory.c b/lib/efi_loader/efi_memory.c
>>index b77c2f980cc..882366a9f8a 100644
>>--- a/lib/efi_loader/efi_memory.c
>>+++ b/lib/efi_loader/efi_memory.c
>>@@ -129,13 +129,13 @@ static uint64_t desc_get_end(struct efi_mem_desc *desc)
>> static void efi_mem_sort(void)
>> {
>> 	struct efi_mem_list *lmem;
>>-	struct efi_mem_list *prevmem = NULL;
>> 	bool merge_again = true;
>> 
>> 	list_sort(NULL, &efi_mem, efi_mem_cmp);
>> 
>> 	/* Now merge entries that can be merged */
>> 	while (merge_again) {
>
> The suggested change looks correct. But we could go further:
>
> We could get rid of this outer loop by either eliminating prev instead of cur or by using list_for_each_safe().
>
> Best regards
>
> Heinrich

Very good point. Didn't know about the safe iteration functions but this logic
would benefit greatly from comparing current and next efi_mem_desc and merging
into the next efi_mem_desc instance instead. Less arithmetic, iteration, and
generally less room for error.

Will apply that.

>>+		struct efi_mem_list *prevmem = NULL;
>> 		merge_again = false;
>> 		list_for_each_entry(lmem, &efi_mem, link) {
>> 			struct efi_mem_desc *prev;