Re: [PATCHv3 4/6] efi_selftest_memory: check for duplicates first

[Randolph Sapp <rs@ti.com>]

On Fri Apr 17, 2026 at 3:17 AM CDT, Ilias Apalodimas wrote:
> On Thu, 16 Apr 2026 at 23:26, Randolph Sapp <rs@ti.com> wrote:
>>
>> On Thu Apr 16, 2026 at 3:55 AM CDT, Ilias Apalodimas wrote:
>> > On Mon, 13 Apr 2026 at 23:36, <rs@ti.com> wrote:
>> >>
>> >> From: Randolph Sapp <rs@ti.com>
>> >>
>> >> Check for duplicate memory mappings before reporting any incorrect
>> >> attributes. Could be that second allocation has the correct type while
>> >> the first doesn't. Knowing there is a duplicate in this scenario is
>> >> more helpful than just reporting the first mismatch.
>> >>
>> >> Signed-off-by: Randolph Sapp <rs@ti.com>
>> >
>> > [...]
>> >
>> >>                         }
>> >> -                       found = true;
>> >> -                       if (memory_type != entry->type) {
>> >> -                               efi_st_error
>> >> -                                       ("Wrong memory type %d, expected %d\n",
>> >> -                                        entry->type, memory_type);
>> >> -                               return EFI_ST_FAILURE;
>> >> -                       }
>> >> +                       match = entry;
>> >>                 }
>> >>         }
>> >> -       if (!found) {
>> >> +       if (!match) {
>> >>                 efi_st_error("Missing memory map entry\n");
>> >>                 return EFI_ST_FAILURE;
>> >>         }
>> >> +       if (memory_type != match->type) {
>> >> +               efi_st_error("Wrong memory type %d, expected %d\n", match->type,
>> >> +                            memory_type);
>> >> +               return EFI_ST_FAILURE;
>> >
>> > This check is now outside the loop and only checks for the last entry.
>> > If you wan't to split the fucntionality, don't we need a loop over all
>> > memory areas and the type?
>> >
>> > Thanks
>> > /Ilias
>>
>> Not necessarily. At the end of the day we can only really raise one exception
>> anyway. I just think informing the user about a duplicate should take priority
>> above mismatched attributes. It hints at a bigger issue.
>
> I dont mind about the priority. We can swap that over. But with this
> patch we will check for less problems than we currently do.
>
> Thanks
> /Ilias

In what regards? We're still checking for mismatched attributes, we just check
for duplicates first. If there's a dupe it doesn't really matter what the
attributes are on either allocation, good or bad something horrible has
occurred.

The prior version of this would never report if there was a duplicate unless the
first allocation it saw had the correct attributes.

>>
>> >> +       }
>> >>         return EFI_ST_SUCCESS;
>> >>  }
>> >>
>> >> --
>> >> 2.53.0
>> >>
>>