[PATCH v2 1/1] efi_loader: simplify efi_uninstall_protocol()

[PATCH v2 1/1] efi_loader: simplify efi_uninstall_protocol()

[Heinrich Schuchardt]

The call to efi_search_obj() is redundant as the function is called in
efi_search_protocol() too.

Signed-off-by: Heinrich Schuchardt <heinrich.schuchardt@canonical.com>
---
v2:
	Use handle instead of efiobj to invoke efi_disconnect_all_drivers()
---
 lib/efi_loader/efi_boottime.c | 9 +--------
 1 file changed, 1 insertion(+), 8 deletions(-)

diff --git a/lib/efi_loader/efi_boottime.c b/lib/efi_loader/efi_boottime.c
index d5065f296a..00f1dd0f56 100644
--- a/lib/efi_loader/efi_boottime.c
+++ b/lib/efi_loader/efi_boottime.c
@@ -1336,24 +1336,17 @@ static efi_status_t efi_uninstall_protocol
 			(efi_handle_t handle, const efi_guid_t *protocol,
 			 void *protocol_interface)
 {
-	struct efi_object *efiobj;
 	struct efi_handler *handler;
 	struct efi_open_protocol_info_item *item;
 	struct efi_open_protocol_info_item *pos;
 	efi_status_t r;
 
-	/* Check handle */
-	efiobj = efi_search_obj(handle);
-	if (!efiobj) {
-		r = EFI_INVALID_PARAMETER;
-		goto out;
-	}
 	/* Find the protocol on the handle */
 	r = efi_search_protocol(handle, protocol, &handler);
 	if (r != EFI_SUCCESS)
 		goto out;
 	/* Disconnect controllers */
-	efi_disconnect_all_drivers(efiobj, protocol, NULL);
+	efi_disconnect_all_drivers(handle, protocol, NULL);
 	/* Close protocol */
 	list_for_each_entry_safe(item, pos, &handler->open_infos, link) {
 		if (item->info.attributes ==
-- 
2.40.1

Re: [PATCH v2 1/1] efi_loader: simplify efi_uninstall_protocol()

[Ilias Apalodimas]

Hi Heinrich 

On Sun, Jun 18, 2023 at 12:25:47PM +0200, Heinrich Schuchardt wrote:
> The call to efi_search_obj() is redundant as the function is called in
> efi_search_protocol() too.
> 
> Signed-off-by: Heinrich Schuchardt <heinrich.schuchardt@canonical.com>
> ---
> v2:
> 	Use handle instead of efiobj to invoke efi_disconnect_all_drivers()
> ---
>  lib/efi_loader/efi_boottime.c | 9 +--------
>  1 file changed, 1 insertion(+), 8 deletions(-)
> 
> diff --git a/lib/efi_loader/efi_boottime.c b/lib/efi_loader/efi_boottime.c
> index d5065f296a..00f1dd0f56 100644
> --- a/lib/efi_loader/efi_boottime.c
> +++ b/lib/efi_loader/efi_boottime.c
> @@ -1336,24 +1336,17 @@ static efi_status_t efi_uninstall_protocol
>  			(efi_handle_t handle, const efi_guid_t *protocol,
>  			 void *protocol_interface)
>  {
> -	struct efi_object *efiobj;
>  	struct efi_handler *handler;
>  	struct efi_open_protocol_info_item *item;
>  	struct efi_open_protocol_info_item *pos;
>  	efi_status_t r;
>  
> -	/* Check handle */
> -	efiobj = efi_search_obj(handle);
> -	if (!efiobj) {
> -		r = EFI_INVALID_PARAMETER;
> -		goto out;
> -	}
>  	/* Find the protocol on the handle */
>  	r = efi_search_protocol(handle, protocol, &handler);
>  	if (r != EFI_SUCCESS)
>  		goto out;
>  	/* Disconnect controllers */
> -	efi_disconnect_all_drivers(efiobj, protocol, NULL);
> +	efi_disconnect_all_drivers(handle, protocol, NULL);

Mind dropping this one ?
I've already sent a fix for this here[0].

You can add my reviewed-by on v1 instead

[0] https://lore.kernel.org/u-boot/20230615143941.416924-3-ilias.apalodimas@linaro.org/

Thanks
/Ilias

>  	/* Close protocol */
>  	list_for_each_entry_safe(item, pos, &handler->open_infos, link) {
>  		if (item->info.attributes ==
> -- 
> 2.40.1
> 

Re: [PATCH v2 1/1] efi_loader: simplify efi_uninstall_protocol()

[Ilias Apalodimas]

On Sun, 18 Jun 2023 at 17:02, Ilias Apalodimas
<ilias.apalodimas@linaro.org> wrote:
>
> Hi Heinrich
>
> On Sun, Jun 18, 2023 at 12:25:47PM +0200, Heinrich Schuchardt wrote:
> > The call to efi_search_obj() is redundant as the function is called in
> > efi_search_protocol() too.
> >
> > Signed-off-by: Heinrich Schuchardt <heinrich.schuchardt@canonical.com>
> > ---
> > v2:
> >       Use handle instead of efiobj to invoke efi_disconnect_all_drivers()
> > ---
> >  lib/efi_loader/efi_boottime.c | 9 +--------
> >  1 file changed, 1 insertion(+), 8 deletions(-)
> >
> > diff --git a/lib/efi_loader/efi_boottime.c b/lib/efi_loader/efi_boottime.c
> > index d5065f296a..00f1dd0f56 100644
> > --- a/lib/efi_loader/efi_boottime.c
> > +++ b/lib/efi_loader/efi_boottime.c
> > @@ -1336,24 +1336,17 @@ static efi_status_t efi_uninstall_protocol
> >                       (efi_handle_t handle, const efi_guid_t *protocol,
> >                        void *protocol_interface)
> >  {
> > -     struct efi_object *efiobj;
> >       struct efi_handler *handler;
> >       struct efi_open_protocol_info_item *item;
> >       struct efi_open_protocol_info_item *pos;
> >       efi_status_t r;
> >
> > -     /* Check handle */
> > -     efiobj = efi_search_obj(handle);
> > -     if (!efiobj) {
> > -             r = EFI_INVALID_PARAMETER;
> > -             goto out;
> > -     }
> >       /* Find the protocol on the handle */
> >       r = efi_search_protocol(handle, protocol, &handler);
> >       if (r != EFI_SUCCESS)
> >               goto out;
> >       /* Disconnect controllers */
> > -     efi_disconnect_all_drivers(efiobj, protocol, NULL);
> > +     efi_disconnect_all_drivers(handle, protocol, NULL);
>
> Mind dropping this one ?
> I've already sent a fix for this here[0].
>
> You can add my reviewed-by on v1 instead
>
> [0] https://lore.kernel.org/u-boot/20230615143941.416924-3-ilias.apalodimas@linaro.org/
>
> Thanks
> /Ilias

As we discussed over IRC, just add this and I'll drop it from my patches

Reviewed-by: Ilias Apalodimas <ilias.apalodimas@linaro.org>

>
> >       /* Close protocol */
> >       list_for_each_entry_safe(item, pos, &handler->open_infos, link) {
> >               if (item->info.attributes ==
> > --
> > 2.40.1
> >