From mboxrd@z Thu Jan  1 00:00:00 1970
From: Marek Vasut <marex@denx.de>
Date: Thu, 28 Jan 2021 20:07:54 +0100
Subject: [PATCH] env: Fix warning when forcing environment without
 ENV_ACCESS_IGNORE_FORCE
In-Reply-To: <1610360847-21890-1-git-send-email-martin.fuzzey@flowbird.group>
References: <1610360847-21890-1-git-send-email-martin.fuzzey@flowbird.group>
Message-ID: <692a008f-22d0-e206-9edb-7a5bad124ad5@denx.de>
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

On 1/11/21 11:27 AM, Martin Fuzzey wrote:
> Since commit 0f036bf4b87e ("env: Warn on force access if ENV_ACCESS_IGNORE_FORCE set")
> a warning message is displayed when setenv -f is used WITHOUT
> CONFIG_ENV_ACCESS_IGNORE_FORCE, but the variable is set anyway, resulting
> in lots of log pollution.
> 
> env_flags_validate() returns 0 if the access is accepted, or non zero
> if it is refused.
> 
> So the original code
> 	#ifndef CONFIG_ENV_ACCESS_IGNORE_FORCE
> 		if (flag & H_FORCE)
> 			return 0;
> 	#endif
> 
> was correct, it returns 0 (accepts the modification) if forced UNLESS
> IGNORE_FORCE is set (in which case access checks in the following code
> are applied). The broken patch just added a printf to the force accepted
> case.
> 
> To obtain the intent of the patch we need this:
> 	if (flag & H_FORCE) {
> 	#ifdef CONFIG_ENV_ACCESS_IGNORE_FORCE
> 		printf("## Error: Can't force access to \"%s\"\n", name);
> 	#else
> 		return 0;
> 	#endif
> 	}
> 
> Fixes: 0f036bf4b87e ("env: Warn on force access if ENV_ACCESS_IGNORE_FORCE set")
> 
> Signed-off-by: Martin Fuzzey <martin.fuzzey@flowbird.group>
> ---
>   env/flags.c | 5 +++--
>   1 file changed, 3 insertions(+), 2 deletions(-)
> 
> diff --git a/env/flags.c b/env/flags.c
> index df4aed2..e3e833c 100644
> --- a/env/flags.c
> +++ b/env/flags.c
> @@ -563,12 +563,13 @@ int env_flags_validate(const struct env_entry *item, const char *newval,
>   		return 1;
>   #endif
>   
> -#ifndef CONFIG_ENV_ACCESS_IGNORE_FORCE
>   	if (flag & H_FORCE) {
> +#ifdef CONFIG_ENV_ACCESS_IGNORE_FORCE
>   		printf("## Error: Can't force access to \"%s\"\n", name);
> +#else
>   		return 0;
> -	}
>   #endif

Based on env/Kconfig  description of this option:

config ENV_ACCESS_IGNORE_FORCE
         bool "Block forced environment operations"
         default n
         help
           If defined, don't allow the -f switch to env set override 
variable
           access flags.

I would think the code should look like this:

#ifdef CONFIG_ENV_ACCESS_IGNORE_FORCE
         if (flag & H_FORCE) {
                 printf("## Error: Can't force access to \"%s\"\n", name);
                 return 1;
         }
#else
         if (flag & H_FORCE)
		return 0;
#endif