[U-Boot] [RFC PATCH] u-boot: remove driver lookup loop from env_save()

[Simon Glass]

Hi Nicholas,

On 10 July 2018 at 06:57, Nicholas Faustini
<nicholas.faustini at azcomtech.com> wrote:
> When called with ENVOP_SAVE, env_get_location() only returns the
> gd->env_load_location variable without actually checking for
> the environment location and priority. This allows saving the
> environment into the same location that has been previously loaded.
>
> This behaviour causes env_save() to fall into an infinite loop when
> the low-level drv->save() call fails.

Why is this? Should it not stop eventually? Do we need a limit on prio?

>
> The env_save() function should not loop through the environment
> location list but it should use the previously discovered
> environment driver once.

What is that? It should be possible to write the env to multiple
places. Also what is the previously discovered environment?

>
> Signed-off-by: Nicholas Faustini <nicholas.faustini at azcomtech.com>
> ---
>
>  env/env.c | 8 ++++----
>  1 file changed, 4 insertions(+), 4 deletions(-)
>
> diff --git a/env/env.c b/env/env.c
> index 5c0842a..897d197 100644
> --- a/env/env.c
> +++ b/env/env.c
> @@ -211,16 +211,16 @@ int env_load(void)
>  int env_save(void)
>  {
>         struct env_driver *drv;
> -       int prio;
>
> -       for (prio = 0; (drv = env_driver_lookup(ENVOP_SAVE, prio)); prio++) {
> +       drv = env_driver_lookup(ENVOP_SAVE, 0);
> +       if (drv) {
>                 int ret;
>
>                 if (!drv->save)
> -                       continue;
> +                       return -ENODEV;
>
>                 if (!env_has_inited(drv->location))
> -                       continue;
> +                       return -ENODEV;
>
>                 printf("Saving Environment to %s... ", drv->name);
>                 ret = drv->save();
> --
> 2.7.4
>

Regards,
Simon