From mboxrd@z Thu Jan  1 00:00:00 1970
From: Alex Kiernan <alex.kiernan@gmail.com>
Date: Wed, 26 Aug 2020 15:29:05 +0100
Subject: [PATCH V2 1/7] env: Warn on force access if
 ENV_ACCESS_IGNORE_FORCE set
In-Reply-To: <20200707185139.2225-1-marex@denx.de>
References: <20200707185139.2225-1-marex@denx.de>
Message-ID: <CAO5Uq5RGBkqAr+rFgPuJ-BP=Oi7nmq4025AMra4hNyvQUFDXUA@mail.gmail.com>
List-Id: <u-boot.lists.denx.de>
MIME-Version: 1.0
Content-Type: text/plain; charset="us-ascii"
Content-Transfer-Encoding: 7bit
To: u-boot@lists.denx.de

On Tue, Jul 7, 2020 at 7:52 PM Marek Vasut <marex@denx.de> wrote:
>
> If the ENV_ACCESS_IGNORE_FORCE is set, inform user that the variable
> cannot be force-set if such attempt happens.
>
> Signed-off-by: Marek Vasut <marex@denx.de>
> ---
> V2: No change
> ---
>  env/flags.c | 4 +++-
>  1 file changed, 3 insertions(+), 1 deletion(-)
>
> diff --git a/env/flags.c b/env/flags.c
> index b88fe7ba9c..f7a53775c4 100644
> --- a/env/flags.c
> +++ b/env/flags.c
> @@ -524,8 +524,10 @@ int env_flags_validate(const struct env_entry *item, const char *newval,
>
>         /* check for access permission */
>  #ifndef CONFIG_ENV_ACCESS_IGNORE_FORCE
> -       if (flag & H_FORCE)
> +       if (flag & H_FORCE) {
> +               printf("## Error: Can't force access to \"%s\"\n", name);
>                 return 0;
> +       }
>  #endif
>         switch (op) {
>         case env_op_delete:

AFAICT this is wrong - you get the message when you have
CONFIG_ENV_ACCESS_IGNORE_FORCE disabled and use force:

      => env print ethaddr
      ethaddr=00:1C:2B:08:AF:65
      => env set ethaddr 00:00:00:00:00:00
      ## Error: Can't overwrite "ethaddr"
      ## Error inserting "ethaddr" variable, errno=1
      => env print ethaddr
      ethaddr=00:1C:2B:08:AF:65
      => env set -f ethaddr 00:00:00:00:00:00
      ## Error: Can't force access to "ethaddr"
      => env print ethaddr
      ethaddr=00:00:00:00:00:00

Just staring at the code, I don't see a good way to capture this
behaviour, other than wiring it into each of the branches of the
switch - I started off with something like this:

diff --git a/env/flags.c b/env/flags.c
index df4aed26b2c6..70621dff4434 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) {
-               printf("## Error: Can't force access to \"%s\"\n", name);
-               return 0;
+               if (CONFIG_IS_ENABLED(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) {

But I think with that you'll get the message for variables which can
be overwritten, so still not what's intended.

--
Alex Kiernan