[U-Boot] [RFC, PATCH v4 11/16] env: save a context immediately if 'autosave' variable is changed

[Wolfgang Denk]

Dear Takahiro,

In message <20190717082525.891-12-takahiro.akashi at linaro.org> you wrote:
> By definition, when any variable with VARSTORAGE_NON_VOLATILE_AUTOSAVE
> is modified with env_save_ext(), the associated context should be written
> back to storage immediately.
>
> Signed-off-by: AKASHI Takahiro <takahiro.akashi at linaro.org>
> ---
>  cmd/nvedit.c | 34 ++++++++++++++++++++++++++++++++--
>  1 file changed, 32 insertions(+), 2 deletions(-)
>
> diff --git a/cmd/nvedit.c b/cmd/nvedit.c
> index cc80ba712767..9896a4141a2a 100644
> --- a/cmd/nvedit.c
> +++ b/cmd/nvedit.c
> @@ -271,8 +271,27 @@ static int _do_env_set(int flag, int argc, char * const argv[], int env_flag,
>  
>  	/* Delete only ? */
>  	if (argc < 3 || argv[2] == NULL) {
> -		int rc = hdelete_r(name, &env_htab, env_flag);
> -		return !rc;
> +		uint32_t flags;
> +
> +		int rc = hdelete_ext(name, &env_htab, ctx, &flags, env_flag);
> +		if (!rc) {
> +			debug("Failed to delete from hash table\n");
> +			return 1;
> +		}
> +
> +		if (env_flags_parse_varstorage_from_binflags(flags)
> +				== env_flags_varstorage_non_volatile_autosave) {
> +			int ret;
> +
> +			ret = env_save_ext(ctx);
> +			if (ret) {
> +				printf("## Error saving variables, ret = %d\n",
> +				       ret);
> +				return 1;
> +			}
> +		}
> +
> +		return 0;

I think your logic of using hdelete() here to provide flag
information is not correct.  The existing code stores flag
information independent of the actual existence of the variable.
Your code breaks this.

Best regards,

Wolfgang Denk

-- 
DENX Software Engineering GmbH,      Managing Director: Wolfgang Denk
HRB 165235 Munich, Office: Kirchenstr.5, D-82194 Groebenzell, Germany
Phone: (+49)-8142-66989-10 Fax: (+49)-8142-66989-80 Email: wd at denx.de
Quantum Mechanics is God's version of "Trust me."