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

Marek Vasut
2021-01-29T11:16:02Z

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 ?)