[PATCH 4/6] efi_memory: nitpick removal loop

[Ilias Apalodimas]

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
>
> > 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
> >>
>