[PATCH v2 2/3] efi: Convert device_path allocation to use malloc()

[Ilias Apalodimas]

On Thu, 29 Aug 2024 at 15:17, Simon Glass <sjg at chromium.org> wrote:
>
> Hi Ilias,
>
> On Wed, 28 Aug 2024 at 01:40, Ilias Apalodimas
> <ilias.apalodimas at linaro.org> wrote:
> >
> > Hi Simon,
> >
> > [...]
> >
> > > > > > > > ---
> > > > > > > >
> > > > > > > > (no changes since v1)
> > > > > > >
> > > > > > > What about https://lore.kernel.org/u-boot/CAFLszTjLAKaYK_jLXJ7Z571L-QMtoiqE-oxHCRs186dZ2Qok5g@mail.gmail.com/
> > > > > > > ?
> > > > > >
> > > > > > Yes, I reordered the patches in this series.
> > > > >
> > > > > You don't need to reorder them. As Heinrich already pointed out some
> > > > > of these functions are used in EFI protocols. e.g
> > > > > duplicate_device_path() requires the memory to be allocated by EFI
> > > > > memory services, you can't just replace it with malloc.
> > > >
> > > > The pointers returned can be freed with EFI services. I don't see any
> > > > problem here. All the tests pass and all the spec rules are followed,
> > > > so far as I can tell.
> > >
> > > Where exactly does that happen in the current patch?
> > >
> > > efi_alloc() calls efi_allocate_pool() and when we need to free the
> > > device path we are calling efi_free_pool()
> > >
> >
> > Looking at it again, this wasn't very clear. The previous patch converts
> > efi_allocate_pool() to use malloc. So this patch is not needed at all
>
> That's right, but ultimately I'd like to drop efi_alloc() and the only
> other place it is used is once in lib/efi_loader/efi_bootmgr.c

Right, then the right thing to do here is to replace efi_alloc() with
efi_allocate_pool() which will be calling malloc in the end. That way
we are free to change the efi_allocate_pool implementation in the
future without affecting the functions described on the spec

Thanks
/Ilias


>
> Regards,
> Simon