grub-devel
[Top][All Lists]
Advanced

[Date Prev][Date Next][Thread Prev][Thread Next][Date Index][Thread Index]

Re: [PATCH v1 37/37] chainloader: Use grub_efi_...() memory helpers wher


From: Leo Sandoval
Subject: Re: [PATCH v1 37/37] chainloader: Use grub_efi_...() memory helpers where reasonable.
Date: Thu, 10 Oct 2024 12:26:27 -0600



On Mon, Oct 7, 2024 at 1:44 PM <ross.philipson@oracle.com> wrote:
On 10/7/24 11:21 AM, Leo Sandoval wrote:
> From: Peter Jones <pjones@redhat.com>
>
> This uses grub_efi_allocate_pool(), grub_efi_free_pool(), and
> grub_efi_free_pages() instead of open-coded efi_call_N() calls, so we
> get more reasonable type checking.

While the idea of putting wrappers around the EFI pool allocation calls
seems reasonable (re: previous patch), this patch does not do what the
comment says. Nothing seems to call grub_efi_allocate_pool(). But since
something freed a pool in grub_chainloader_boot(), it must have been
allocated somewhere. Was that part left out?

you are right in both points, comment is not precise and there are other places where grub_efi_allocate_pool()
wrapper can be used but it is left out. Adding the allocate wrapper deserves a separate patch, For the moment,
I will update the comment and bump the version.


Thanks
Ross

>
> Signed-off-by: Peter Jones <pjones@redhat.com>
> ---
>   grub-core/loader/efi/chainloader.c | 4 ++--
>   1 file changed, 2 insertions(+), 2 deletions(-)
>
> diff --git a/grub-core/loader/efi/chainloader.c b/grub-core/loader/efi/chainloader.c
> index 1de98f783..203692450 100644
> --- a/grub-core/loader/efi/chainloader.c
> +++ b/grub-core/loader/efi/chainloader.c
> @@ -95,7 +95,7 @@ grub_chainloader_boot (void *context)
>       }
>   
>     if (exit_data)
> -    b->free_pool (exit_data);
> +    grub_efi_free_pool (exit_data);
>   
>     grub_loader_unset ();
>   
> @@ -419,7 +419,7 @@ grub_cmd_chainloader (grub_command_t cmd __attribute__ ((unused)),
>     grub_free (file_path);
>   
>     if (address)
> -    b->free_pages (address, pages);
> +    grub_efi_free_pages (address, pages);
>   
>     if (image_handle != NULL)
>       b->unload_image (image_handle);


reply via email to

[Prev in Thread] Current Thread [Next in Thread]