From mboxrd@z Thu Jan  1 00:00:00 1970
From: Simon Glass <sjg@chromium.org>
Date: Thu, 31 Aug 2017 20:51:20 +0800
Subject: [U-Boot] [PATCH 04/23] efi_loader: rework efi_locate_handle
In-Reply-To: <20170826225110.7381-5-xypron.glpk@gmx.de>
References: <20170826225110.7381-1-xypron.glpk@gmx.de>
 <20170826225110.7381-5-xypron.glpk@gmx.de>
Message-ID: <CAPnjgZ3RW8jvV1yVuRfLgNmwo5XD4TRLGh2SYYLE7d+b1Gx8LA@mail.gmail.com>
List-Id: <u-boot.lists.denx.de>
MIME-Version: 1.0
Content-Type: text/plain; charset="us-ascii"
Content-Transfer-Encoding: 7bit
To: u-boot@lists.denx.de

Hi Heinrich,

On 27 August 2017 at 06:51, Heinrich Schuchardt <xypron.glpk@gmx.de> wrote:
> Check the parameters in efi_locate_handle.
>
> Use list_for_each_entry instead of list_for_each.
>
> Signed-off-by: Heinrich Schuchardt <xypron.glpk@gmx.de>
> ---
>  lib/efi_loader/efi_boottime.c | 42 +++++++++++++++++++++++++++++++-----------
>  1 file changed, 31 insertions(+), 11 deletions(-)
>

Reviewed-by: Simon Glass <sjg@chromium.org>

nits below

> diff --git a/lib/efi_loader/efi_boottime.c b/lib/efi_loader/efi_boottime.c
> index b5538e0769..570a5ea186 100644
> --- a/lib/efi_loader/efi_boottime.c
> +++ b/lib/efi_loader/efi_boottime.c
> @@ -599,6 +599,7 @@ static int efi_search(enum efi_locate_search_type search_type,
>         case all_handles:
>                 return 0;
>         case by_register_notify:
> +               /* RegisterProtocolNotify is not implemented yet */
>                 return -1;
>         case by_protocol:
>                 for (i = 0; i < ARRAY_SIZE(efiobj->protocols); i++) {
> @@ -617,16 +618,38 @@ static efi_status_t efi_locate_handle(
>                         efi_guid_t *protocol, void *search_key,
>                         unsigned long *buffer_size, efi_handle_t *buffer)

function needs a comment

>  {
> -       struct list_head *lhandle;
> +       struct efi_object *efiobj;
>         unsigned long size = 0;
>
> +       /* Check parameters */
> +       switch (search_type) {
> +       case all_handles:
> +               break;
> +       case by_register_notify:
> +               if (!search_key)
> +                       return EFI_INVALID_PARAMETER;
> +               /* RegisterProtocolNotify is not implemented yet */
> +               return EFI_UNSUPPORTED;
> +       case by_protocol:
> +               if (!protocol)
> +                       return EFI_INVALID_PARAMETER;
> +               break;
> +       default:
> +               return EFI_INVALID_PARAMETER;
> +       }
> +
> +       /*
> +        * efi_locate_handle_buffer uses this function for
> +        * the calculation of the necessary buffer size.
> +        * So do not require a buffer for buffersize == 0.
> +        */
> +       if (!buffer_size || (*buffer_size && !buffer))
> +               return EFI_INVALID_PARAMETER;
> +
>         /* Count how much space we need */
> -       list_for_each(lhandle, &efi_obj_list) {
> -               struct efi_object *efiobj;
> -               efiobj = list_entry(lhandle, struct efi_object, link);
> -               if (!efi_search(search_type, protocol, search_key, efiobj)) {
> +       list_for_each_entry(efiobj, &efi_obj_list, link) {
> +               if (!efi_search(search_type, protocol, search_key, efiobj))
>                         size += sizeof(void*);
> -               }
>         }
>
>         if (*buffer_size < size) {
> @@ -639,12 +662,9 @@ static efi_status_t efi_locate_handle(
>                 return EFI_NOT_FOUND;
>
>         /* Then fill the array */
> -       list_for_each(lhandle, &efi_obj_list) {
> -               struct efi_object *efiobj;
> -               efiobj = list_entry(lhandle, struct efi_object, link);
> -               if (!efi_search(search_type, protocol, search_key, efiobj)) {
> +       list_for_each_entry(efiobj, &efi_obj_list, link) {
> +               if (!efi_search(search_type, protocol, search_key, efiobj))
>                         *(buffer++) = efiobj->handle;

*buffer++

> -               }
>         }
>
>         return EFI_SUCCESS;
> --
> 2.14.1
>

Regards,
Simon