On 12.07.2018 12:52, Nicholas Faustini 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 behaviour causes env_save() to fall into an infinite loop when the low-level drv->save() call fails.
The env_save() function should not loop through the environment location list but it should save the environment into the location stored in gd->env_load_location by the last env_load() call.
Signed-off-by: Nicholas Faustini nicholas.faustini@azcomtech.com
Changes in v2:
Restore gd->env_load_location to the highest priority location when env_load() fails
env/env.c | 10 ++++++---- 1 file changed, 6 insertions(+), 4 deletions(-)
diff --git a/env/env.c b/env/env.c index 5c0842a..18eb78d 100644 --- a/env/env.c +++ b/env/env.c @@ -205,22 +205,24 @@ int env_load(void) return 0; }
- env_get_location(ENVOP_LOAD, 0);
A comment why this is required would be good, I guess.
return -ENODEV; }
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);
Thinking again about this, would it make more sense to store 'env_load_prio' in 'gd' after successful load? That way, 'env_get_location()' would be more straightforward (no special case for ENVOP_SAVE) and here in 'env_save()' we could just write something like this:
drv = env_driver_lookup(ENVOP_SAVE, gd->env_load_prio);
Simon
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();