Re: [PATCH 09/15] efi_loader: Fix free in ..._media_device_boot_option()

[Heinrich Schuchardt]

Am 29. Oktober 2024 16:45:29 MEZ schrieb Simon Glass <s...@chromium.org>:
>Hi Ilias,
>
>On Tue, 29 Oct 2024 at 11:02, Ilias Apalodimas
><ilias.apalodi...@linaro.org> wrote:
>>
>> Hi Simon,
>>
>> On Mon, 28 Oct 2024 at 14:48, Simon Glass <s...@chromium.org> wrote:
>> >
>> > Freeing a NULL pointer is an error in EFI, so check the pointer first,
>> > before freeing it.
>> >
>> > Signed-off-by: Simon Glass <s...@chromium.org>
>> > ---
>> >
>> >  lib/efi_loader/efi_bootmgr.c | 3 ++-
>> >  1 file changed, 2 insertions(+), 1 deletion(-)
>> >
>> > diff --git a/lib/efi_loader/efi_bootmgr.c b/lib/efi_loader/efi_bootmgr.c
>> > index a3aa2b8d1b9..431a38704e9 100644
>> > --- a/lib/efi_loader/efi_bootmgr.c
>> > +++ b/lib/efi_loader/efi_bootmgr.c
>> > @@ -1180,7 +1180,8 @@ out:
>> >                         free(opt[i].lo);
>> >         }
>> >         free(opt);
>> > -       efi_free_pool(handles);
>> > +       if (handles)
>> > +               efi_free_pool(handles);
>>
>> We don't need this, efi_free_pool() checks the pointer already.
>
>Yes, but it then returns an error (EFI_INVALID_PARAMETER), which gets
>logged, with this series.

So this is not a problem of the existing code but of your patch series which 
creates a superfluous log message.

Best regards

Heinrich

>
>Regards,
>Simon