All of lore.kernel.org
 help / color / mirror / Atom feed
From: Marek Vasut <marex@denx.de>
To: u-boot@lists.denx.de
Subject: [PATCH] env: Fix warning when forcing environment without ENV_ACCESS_IGNORE_FORCE
Date: Fri, 29 Jan 2021 12:16:02 +0100	[thread overview]
Message-ID: <67976a0a-e211-dad7-82fa-7a2bbeb862d6@denx.de> (raw)
In-Reply-To: <CANh8QzwAZ9cOun3U2Cgekx0S=SyGWjywVL9VPRL=ZkpFZLRwXA@mail.gmail.com>

On 1/29/21 8:42 AM, Fuzzey, Martin wrote:
> On Fri, 29 Jan 2021 at 00:05, Marek Vasut <marex@denx.de> wrote:
>>
>> On 1/28/21 8:37 PM, Fuzzey, Martin wrote:
>>> Hi Marek,
>>
>> Hi,
>>
>>> On Thu, 28 Jan 2021 at 20:07, Marek Vasut <marex@denx.de> wrote:
>>>>
>>>> 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
>>>
>>> I don't think that is right.
>>>
>>> If you do that force update will be refused when
>>> CONFIG_ENV_ACCESS_IGNORE_FORCE, regardless if the access would be
>>> valid without the force
>>
>> That's how I understand the option was intended to work, based on the
>> Kconfig help text.
>>
> 
> Ok we are understanding different things about what it's supposed to do then.
> 
> But if everyone agrees that the option should completely disable -f
> then I'm fine with that, in which case the version of the patch you
> propose is good (and easier to understand).
> 
> The problem I have is with the current 0f036bf4b87e which leads to log
> spam on boards that *don't* have CONFIG_ENV_ACCESS_IGNORE_FORCE set
> and do use -f.

Yes, that should be fixed, one way or the other.
(in fact, can you also write a test for this case ?)

  reply	other threads:[~2021-01-29 11:16 UTC|newest]

Thread overview: 15+ messages / expand[flat|nested]  mbox.gz  Atom feed  top
2021-01-11 10:27 [PATCH] env: Fix warning when forcing environment without ENV_ACCESS_IGNORE_FORCE Martin Fuzzey
2021-01-15 18:43 ` Tom Rini
2021-01-28 16:25   ` Tom Rini
2021-01-28  9:11 ` Heinrich Schuchardt
2021-01-28 18:59   ` Marek Vasut
2021-01-28 19:07 ` Marek Vasut
2021-01-28 19:26   ` Tom Rini
2021-01-28 23:03     ` Marek Vasut
2021-02-01 19:31       ` Tom Rini
2021-02-03 16:39         ` Marek Vasut
2021-01-28 19:37   ` Fuzzey, Martin
2021-01-28 23:05     ` Marek Vasut
2021-01-29  7:42       ` Fuzzey, Martin
2021-01-29 11:16         ` Marek Vasut [this message]
2021-04-18 12:45 ` Tom Rini

Reply instructions:

You may reply publicly to this message via plain-text email
using any one of the following methods:

* Save the following mbox file, import it into your mail client,
  and reply-to-all from there: mbox

  Avoid top-posting and favor interleaved quoting:
  https://en.wikipedia.org/wiki/Posting_style#Interleaved_style

* Reply using the --to, --cc, and --in-reply-to
  switches of git-send-email(1):

  git send-email \
    --in-reply-to=67976a0a-e211-dad7-82fa-7a2bbeb862d6@denx.de \
    --to=marex@denx.de \
    --cc=u-boot@lists.denx.de \
    /path/to/YOUR_REPLY

  https://kernel.org/pub/software/scm/git/docs/git-send-email.html

* If your mail client supports setting the In-Reply-To header
  via mailto: links, try the mailto: link
Be sure your reply has a Subject: header at the top and a blank line before the message body.
This is an external index of several public inboxes,
see mirroring instructions on how to clone and mirror
all data and code used by this external index.