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

[Joe Hershberger]

On Wed, Nov 30, 2016 at 4:08 AM, Bernhard Nortmann
<bernhard.nortmann at web.de> wrote:
> Hi Joe!
>
> Thanks for chiming in, especially seeing that you have previously worked
> on something very similar.

Sure! Sorry I didn't review this sooner.

> Am 27.11.2016 um 19:53 schrieb Joe Hershberger:
>>
>> On Wed, Nov 16, 2016 at 4:29 AM, Bernhard Nortmann
>> <bernhard.nortmann at 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
>>
>> The flags are positional, so 's' is not in use. It seems it would be
>> cleaner to use a lower-case 's'.
>
>
> You're probably right. I think back then I deliberately picked 'S' to avoid
> potential confusion (e.g. users specifying "s" when they actuall mean/want
> "ss"), as trying to set flags to just "S" would result in an error message.
>
> For precisely that reason I'd actually prefer to find letter(s) that would
> not conflict with existing type or access flags, but 'v'olatile seemed
> ambiguous / too broad at that time.

Yes, that is a good point and was actually a consideration when I
first chose the letters that are used for the current 2 positions.
It's not a strict requirement, but it's less error-prone.

>>> [...]
>>
>> I'm not sure why you are adding "transient" or "volatile" to the
>> varaccess. It is an orthogonal property of a variable. This is obvious
>> from the fact that you need to add yet another to compose varaccess
>> with varlifetime (or something).
>>
>> I worked on something similar years ago, but never posted an RFC.
>>
>> http://lists.denx.de/pipermail/u-boot/2010-June/073027.html
>
>
> That's a very good point. This 'orthogonality' is what actually caused me
> to come up with that "transient" vs. "system" idea. I think I got misled
> by the way that U-Boot already combined various "access" bit flags, and it
> never occured to me to introduce another property with a third flag
> character.
> Actually "?av" (any-access, volatile) and  "?rv" (read-only, volatile) would
> represent my intent perfectly well.

Perfect. That's exactly what I was proposing in 2010 (with the
addition of auto-volatile).

> I also like your "auto-volatile" concept a lot (i.e. resetting the volatile
> nature on setenv by the user). To stay unambiguous ('a' = "any" access),
> maybe this could be tagged 't'emporary.

't'emporary is ok, but I'm thinking something like o'v'erride(able) is
more explicit about the behavior.

> We'd also need a default letter for
> the lifesspan flag, possibly 'n'ormal?

I had suggested 'p'recious, but 'n'ormal is ok too. WD wanted a
pointer type too, but I never got around to implementing it.
Presumably that would conflict with 'p'recious if we went with that.

Again, it's not that important that the letters used be unique across
positions, it's just less error prone. At the same time, using the
first letter of a meaning more often will be less error-prone also.

> The temporary (= auto-volatile) flag would also nicely save use from the
> need
> to have users fumble with "setenv .flags", and the quirks involved.

Exactly. It should be the uncommon case to need to touch the .flags.

> Implementing this means some refactoring / a major overhaul of this RFC
> series, but I think it could be well worth that. I'll definitely give it
> a try when I find some time.

Awesome, thanks.
-Joe