On 1/28/21 8:26 PM, Tom Rini wrote:
On Thu, Jan 28, 2021 at 08:07:54PM +0100, Marek Vasut wrote:
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
So, prior to 0f036bf4b87e we had what you're suggesting, and that lead
to 8a5cdf601f8d (which is the commit I was trying to think of) which
Heinrich did not like, but was what was needed to get things to function
again. Wouldn't what you're proposing break the use case you had in the
first place?
No, the idea is to completely block the -f flag if
CONFIG_ENV_ACCESS_IGNORE_FORCE is set from setting anything in the
environment. That's how I understand the Kconfig entry help text.
