[PATCH] env: Fix warning when forcing environment without ENV_ACCESS_IGNORE_FORCE

Tom Rini
2021-01-28T16:25:36Z

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
-------------- next part --------------
A non-text attachment was scrubbed...
Name: signature.asc
Type: application/pgp-signature
Size: 659 bytes
Desc: not available
URL: <https://lists.denx.de/pipermail/u-boot/attachments/20210128/947c0eef/attachment.sig>