[PATCH 4/6] efi_memory: nitpick removal loop

[Randolph Sapp]

On Thu Apr 2, 2026 at 2:02 PM CDT, Ilias Apalodimas wrote:
> On Thu, 2 Apr 2026 at 20:39, Randolph Sapp <rs at ti.com> wrote:
>>
>> On Thu Apr 2, 2026 at 4:04 AM CDT, Ilias Apalodimas wrote:
>> > I don't mind merging this, but why is the current code flawed?
>> >
>> > Looking at the function even if prevmem contains a previous valid
>> > entry the check will fail
>> >
>> > if ((desc_get_end(cur) == prev->physical_start) &&
>> >     (prev->type == cur->type) &&
>> >     (prev->attribute == cur->attribute)) {
>> >
>> > and re-assign prevmem.
>> >
>> > So please adjust the commit message if you want this merged.
>> >
>> > Thanks
>> > /Ilias
>>
>> Ah, not really that it is "flawed" right now, but more so that we should never
>> even have to think about that occurrence, because at best it's an unnecessary
>> compare. Why waste time trying to defend or attack a line of code that simply
>> shouldn't exist?
>
> Not attacking anything. But the commit message wasn't as clear and I
> ended up looking all the code. Chances are this is going to happen if
> someone reads that commit message in a few moths. So something along
> the lines of "This not currently a problem but ...." etc is what we
> should have in that commit.
>
> Cheers
> /Ilias

I didn't mean for it to sound as if you were attacking it. Change requests (like
this patch) are always the "attack." I'm just being lazy and saying I didn't
want to parse this and come up with some specific scenario where it could be a
problem.

I'll adjust the description to indicate the *usually* unnecessary comparison
that's occurring right now. Should be a good enough justification on it's own.

>>
>> > On Thu, 2 Apr 2026 at 03:14, <rs at ti.com> wrote:
>> >>
>> >> From: Randolph Sapp <rs at ti.com>
>> >>
>> >> I don't even want to think about the possibility of this pointer
>> >> containing a reference to something from a previous iteration.
>> >>
>> >> Signed-off-by: Randolph Sapp <rs at 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) {
>> >> +               struct efi_mem_list *prevmem = NULL;
>> >>                 merge_again = false;
>> >>                 list_for_each_entry(lmem, &efi_mem, link) {
>> >>                         struct efi_mem_desc *prev;
>> >> --
>> >> 2.53.0
>> >>
>>