On 11.01.21 11:27, 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,
Please, add a Sphinx style function description in include/env_flags.h explaining exactly what this function is meant to do.
Cf. https://www.kernel.org/doc/html/latest/doc-guide/kernel-doc.html#function-do...
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);
If this is an error, why don't you return 1 (failure)? Please, use log_err() for error messages.
+#else return 0;
Why shouldn't the other tests be executed?
Best regards
Heinrich
- }
#endif
- } switch (op) { case env_op_delete: if (item->flags & ENV_FLAGS_VARACCESS_PREVENT_DELETE) {