Hi Ilias, On Thu, 29 Aug 2024 at 06:39, Ilias Apalodimas <ilias.apalodi...@linaro.org> wrote: > > On Thu, 29 Aug 2024 at 15:17, Simon Glass <s...@chromium.org> wrote: > > > > Hi Ilias, > > > > On Wed, 28 Aug 2024 at 01:40, Ilias Apalodimas > > <ilias.apalodi...@linaro.org> wrote: > > > > > > Hi Simon, > > > > > > [...] > > > > > > > > > > > > --- > > > > > > > > > > > > > > > > > > (no changes since v1) > > > > > > > > > > > > > > > > What about > > > > > > > > https://lore.kernel.org/u-boot/caflsztjlakayk_jlxj7z571l-qmtoiqe-oxhcrs186dz2qo...@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
OK, that sounds good and it will make it clearer which things are internal memory allocations and which are presented to the app as things that it can free. The spec doesn't mention the pool, but I assume that is implicit. Regards, Simon
