[U-Boot,RFC,v2,4/7] env: Introduce "transient" and "system" access flags

Hi Simon!

Am 19.11.2016 um 14:47 schrieb Simon Glass:
> Hi Bernhard,
>
> On 16 November 2016 at 03:29, Bernhard Nortmann
> <bernhard.nortmann@web.de> wrote:
>> "transient" (='t') is like "any", but requests that a variable
>> should not be exported (ENV_FLAGS_VARACCESS_PREVENT_EXPORT).
>>
>> "system" (='S') is meant for 'internal' variables that
>> aren't supposed to be changed by the user. It corresponds
>> to "transient" plus "read-only".
>>
>> Signed-off-by: Bernhard Nortmann <bernhard.nortmann@web.de>
>>
>> ---
>>
>> Changes in v2:
>> - Fixed outdated "env_flags_varaccess_lock" to the correct
>>    "env_flags_varaccess_system"
>>
>>   common/env_flags.c  | 11 +++++++++--
>>   include/env_flags.h |  2 ++
>>   2 files changed, 11 insertions(+), 2 deletions(-)
> Reviewed-by: Simon Glass <sjg@chromium.org>
>
> Please see below.
>
> [...]
> Can these flags be shortened? This is not Java :-) Also it might be
> helpful to use the
>
>    [index] = value
>
> syntax so you can see which value it corresponds to?
>
> [...]
> Regards,
> Simon


I like the [index] suggestion, which already gives a version that I find 
a lot easier to read:


  static const char * const env_flags_vartype_names[] = {

As for the shortening of the various flags: The only one I'm introducing
is ENV_FLAGS_VARACCESS_PREVENT_EXPORT (in patch 3/7), following along the
spirit of existing ones - so I might not be the right person to bust them
all?

Regards, B. Nortmann

Hi Bernhard,

On 22 November 2016 at 11:54, Bernhard Nortmann
<bernhard.nortmann@web.de> wrote:
> Hi Simon!
>
> Am 19.11.2016 um 14:47 schrieb Simon Glass:
>>
>> Hi Bernhard,
>>
>> On 16 November 2016 at 03:29, Bernhard Nortmann
>> <bernhard.nortmann@web.de> wrote:
>>>
>>> "transient" (='t') is like "any", but requests that a variable
>>> should not be exported (ENV_FLAGS_VARACCESS_PREVENT_EXPORT).
>>>
>>> "system" (='S') is meant for 'internal' variables that
>>> aren't supposed to be changed by the user. It corresponds
>>> to "transient" plus "read-only".
>>>
>>> Signed-off-by: Bernhard Nortmann <bernhard.nortmann@web.de>
>>>
>>> ---
>>>
>>> Changes in v2:
>>> - Fixed outdated "env_flags_varaccess_lock" to the correct
>>>    "env_flags_varaccess_system"
>>>
>>>   common/env_flags.c  | 11 +++++++++--
>>>   include/env_flags.h |  2 ++
>>>   2 files changed, 11 insertions(+), 2 deletions(-)
>>
>> Reviewed-by: Simon Glass <sjg@chromium.org>
>>
>> Please see below.
>>
>> [...]
>> Can these flags be shortened? This is not Java :-) Also it might be
>> helpful to use the
>>
>>    [index] = value
>>
>> syntax so you can see which value it corresponds to?
>>
>> [...]
>> Regards,
>> Simon
>
>
>
> I like the [index] suggestion, which already gives a version that I find a
> lot easier to read:
>
>
> diff --git a/common/env_flags.c b/common/env_flags.c
> index f39d952..6dea70c 100644
> --- a/common/env_flags.c
> +++ b/common/env_flags.c
> @@ -28,16 +28,22 @@
>  #endif
>
>  static const char env_flags_vartype_rep[] = "sdxb"
> ENV_FLAGS_NET_VARTYPE_REPS;
> -static const char env_flags_varaccess_rep[] = "aroc";
> +static const char env_flags_varaccess_rep[] = "aroctS";
>  static const int env_flags_varaccess_mask[] = {
> -       0,
> -       ENV_FLAGS_VARACCESS_PREVENT_DELETE |
> -               ENV_FLAGS_VARACCESS_PREVENT_CREATE |
> -               ENV_FLAGS_VARACCESS_PREVENT_OVERWR,
> -       ENV_FLAGS_VARACCESS_PREVENT_DELETE |
> -               ENV_FLAGS_VARACCESS_PREVENT_OVERWR,
> -       ENV_FLAGS_VARACCESS_PREVENT_DELETE |
> -               ENV_FLAGS_VARACCESS_PREVENT_NONDEF_OVERWR};
> +       [0] =   0, /* no restrictions */
> +       [1] =   ENV_FLAGS_VARACCESS_PREVENT_DELETE
> +               | ENV_FLAGS_VARACCESS_PREVENT_CREATE
> +               | ENV_FLAGS_VARACCESS_PREVENT_OVERWR,
> +       [2] =   ENV_FLAGS_VARACCESS_PREVENT_DELETE
> +               | ENV_FLAGS_VARACCESS_PREVENT_OVERWR,
> +       [3] =   ENV_FLAGS_VARACCESS_PREVENT_DELETE
> +               | ENV_FLAGS_VARACCESS_PREVENT_NONDEF_OVERWR,
> +       [4] =   ENV_FLAGS_VARACCESS_PREVENT_EXPORT,
> +       [5] =   ENV_FLAGS_VARACCESS_PREVENT_DELETE
> +               | ENV_FLAGS_VARACCESS_PREVENT_CREATE
> +               | ENV_FLAGS_VARACCESS_PREVENT_OVERWR
> +               | ENV_FLAGS_VARACCESS_PREVENT_EXPORT
> +       };
>
>  #ifdef CONFIG_CMD_ENV_FLAGS
>  static const char * const env_flags_vartype_names[] = {
>
> As for the shortening of the various flags: The only one I'm introducing
> is ENV_FLAGS_VARACCESS_PREVENT_EXPORT (in patch 3/7), following along the
> spirit of existing ones - so I might not be the right person to bust them
> all?

Well you could add a separate patch before this one which renames
everything. I don't think anyone else is working in this area.

Regards,
Simon