On Fri, Jan 15, 2021 at 01:43:44PM -0500, Tom Rini wrote: > On Mon, Jan 11, 2021 at 11:27:20AM +0100, 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 > > + } > > switch (op) { > > case env_op_delete: > > if (item->flags & ENV_FLAGS_VARACCESS_PREVENT_DELETE) { > > Marek, does this look right to you? Heinrich, I think this means > there''s a follow-up commit that I made to one of the tests that can > probably be reverted as well? Thanks for digging in to this Martin!
Marek? Heinrich? I really want a little feedback on this patch since I think it addresses a tricky problem. Thanks. -- Tom
signature.asc
Description: PGP signature